Merge pull request #79 from primevprotocol/ckartik/add-bid-transfer

Adds Idempotent Funds movement with Bid storage during pre-confirmation.

contracts/BidderRegistry.sol

@@ -37,14 +37,17 @@ contract BidderRegistry is IBidderRegistry, Ownable, ReentrancyGuard {
     /// @dev Mapping from bidder addresses to their prepayed amount
     mapping(address => uint256) public bidderPrepaidBalances;
 
+    /// @dev Mapping from bidder addresses to their locked amount based on bidID (commitmentDigest)
+    mapping(bytes32 => BidState) public BidPayment;
+
     /// @dev Amount assigned to bidders
     mapping(address => uint256) public providerAmount;
 
     /// @dev Event emitted when a bidder is registered with their prepayed amount
     event BidderRegistered(address indexed bidder, uint256 prepaidAmount);
 
     /// @dev Event emitted when funds are retrieved from a bidder's prepay
-    event FundsRetrieved(address indexed bidder, uint256 amount);
+    event FundsRetrieved(bytes32 indexed commitmentDigest, uint256 amount);
 
     /**
      * @dev Fallback function to revert all calls, ensuring no unintended interactions.
@@ -140,25 +143,32 @@ contract BidderRegistry is IBidderRegistry, Ownable, ReentrancyGuard {
         return bidderPrepaidBalances[bidder];
     }
 
+    function LockBidFunds(bytes32 commitmentDigest, uint64 bid, address bidder) external onlyPreConfirmationEngine(){
+        BidState memory bidState = BidPayment[commitmentDigest];
+        if (bidState.state == State.Undefined) {
+            BidPayment[commitmentDigest] = BidState({
+                bidAmt: bid,
+                state: State.PreConfirmed,
+                bidder: bidder
+            });
+            bidderPrepaidBalances[bidder] -= bid;
+        }
+    }
+
     /**
      * @dev Retrieve funds from a bidder's prepay (only callable by the pre-confirmations contract).
      * @dev reenterancy not necessary but still putting here for precaution
-     * @param bidder The address of the bidder.
-     * @param amt The amount to retrieve from the bidder's prepay.
+     * @param commitmentDigest is the Bid ID that allows us to identify the bid, and prepayment
      * @param provider The address to transfer the retrieved funds to.
      */
     function retrieveFunds(
-        address bidder,
-        uint256 amt,
+        bytes32 commitmentDigest,
         address payable provider
     ) external nonReentrant onlyPreConfirmationEngine {
-        uint256 amount = bidderPrepaidBalances[bidder];
-        require(
-            amount >= amt,
-            "Amount to retrieve bigger than available funds"
-        );
-        bidderPrepaidBalances[bidder] -= amt;
 
+        BidState memory bidState = BidPayment[commitmentDigest];
+        require(bidState.state == State.PreConfirmed, "The bid was not preconfirmed");
+        uint256 amt = bidState.bidAmt;
         uint256 feeAmt = (amt * uint256(feePercent) * PRECISION) / PERCENT;
         uint256 amtMinusFee = amt - feeAmt;
 
@@ -170,7 +180,29 @@ contract BidderRegistry is IBidderRegistry, Ownable, ReentrancyGuard {
 
         providerAmount[provider] += amtMinusFee;
 
-        emit FundsRetrieved(bidder, amount);
+        // TODO(@ckartik): Ensure we throughly test this flow
+        BidPayment[commitmentDigest].state = State.Withdrawn;
+        BidPayment[commitmentDigest].bidAmt = 0;
+
+        emit FundsRetrieved(commitmentDigest, amt);
+    }
+
+    /**
+     * @dev Return funds to a bidder's prepay (only callable by the pre-confirmations contract).
+     * @dev reenterancy not necessary but still putting here for precaution
+     * @param bidID is the Bid ID that allows us to identify the bid, and prepayment
+     */
+    function unlockFunds(bytes32 bidID) external nonReentrant onlyPreConfirmationEngine() {
+        BidState memory bidState = BidPayment[bidID];
+        require(bidState.state == State.PreConfirmed, "The bid was not preconfirmed");
+        uint256 amt = bidState.bidAmt;
+        bidderPrepaidBalances[bidState.bidder] += amt;
+
+
+        BidPayment[bidID].state = State.Withdrawn;
+        BidPayment[bidID].bidAmt = 0;
+
+        emit FundsRetrieved(bidID, amt);
     }
 
     /**
@@ -196,7 +228,7 @@ contract BidderRegistry is IBidderRegistry, Ownable, ReentrancyGuard {
         feeRecipientAmount = 0;
         require(amount > 0, "fee recipient amount Amount is zero");
         (bool successFee, ) = feeRecipient.call{value: amount}("");
-        require(successFee, "Couldn't transfer to fee Recipient");
+        require(successFee, "couldn't transfer to fee Recipient");
     }
 
     function withdrawProviderAmount(
@@ -207,27 +239,27 @@ contract BidderRegistry is IBidderRegistry, Ownable, ReentrancyGuard {
 
         require(amount > 0, "provider Amount is zero");
         (bool success, ) = provider.call{value: amount}("");
-        require(success, "Couldn't transfer to provider");
+        require(success, "couldn't transfer to provider");
     }
 
     function withdrawPrepaidAmount(address payable bidder) external nonReentrant {
         uint256 prepaidAmount = bidderPrepaidBalances[bidder];
         bidderPrepaidBalances[bidder] = 0;
-        require(msg.sender == bidder, "Only bidder can unprepay");
-        require(prepaidAmount > 0, "Provider Prepayd Amount is zero");
+        require(msg.sender == bidder, "only bidder can unprepay");
+        require(prepaidAmount > 0, "bidder prepaid Amount is zero");
 
         (bool success, ) = bidder.call{value: prepaidAmount}("");
-        require(success, "Couldn't transfer prepay to bidder");
+        require(success, "couldn't transfer prepay to bidder");
     }
 
     function withdrawProtocolFee(
         address payable bidder
     ) external onlyOwner nonReentrant {
         uint256 _protocolFeeAmount = protocolFeeAmount;
         protocolFeeAmount = 0;
-        require(_protocolFeeAmount > 0, "In sufficient protocol fee amount");
+        require(_protocolFeeAmount > 0, "insufficient protocol fee amount");
 
         (bool success, ) = bidder.call{value: _protocolFeeAmount}("");
-        require(success, "Couldn't transfer prepay to bidder");
+        require(success, "couldn't transfer prepay to bidder");
     }
 }

contracts/Oracle.sol

@@ -5,6 +5,8 @@ import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
 import {PreConfCommitmentStore} from "./PreConfirmations.sol";
 import {IProviderRegistry} from "./interfaces/IProviderRegistry.sol";
 import {IPreConfCommitmentStore} from './interfaces/IPreConfirmations.sol';
+import {IBidderRegistry} from './interfaces/IBidderRegistry.sol';
+
 
 /// @title Oracle Contract
 /// @author Kartik Chopra
@@ -44,6 +46,8 @@ contract Oracle is Ownable {
     /// @dev Reference to the PreConfCommitmentStore contract interface.
     IPreConfCommitmentStore private preConfContract;
 
+    IBidderRegistry private bidderRegistry;
+
     /**
      * @dev Constructor to initialize the contract with a PreConfirmations contract.
      * @param _preConfContract The address of the pre-confirmations contract.
@@ -52,10 +56,12 @@ contract Oracle is Ownable {
      */
     constructor(
         address _preConfContract,
+        address _bidderRegistry,
         uint256 _nextRequestedBlockNumber,
         address _owner
     ) Ownable() {
         preConfContract = IPreConfCommitmentStore(_preConfContract);
+        bidderRegistry = IBidderRegistry(_bidderRegistry);
         nextRequestedBlockNumber = _nextRequestedBlockNumber;
         _transferOwnership(_owner);
     }
@@ -119,6 +125,16 @@ contract Oracle is Ownable {
         nextRequestedBlockNumber++;
     }
 
+    /**
+        * @dev unlocks funds to the bidders assosciated with BidIDs in the input array.
+        * @param bidIDs The array of BidIDs to unlock funds for.
+     */
+    function unlockFunds(bytes32[] memory bidIDs) external onlyOwner {
+        for (uint256 i = 0; i < bidIDs.length; i++) {
+            bidderRegistry.unlockFunds(bidIDs[i]);
+        }
+    }
+
     /**
      * @dev Internal function to process a commitment, either slashing or rewarding based on the commitment's state.
      * @param commitmentIndex The id of the commitment to be processed.

contracts/PreConfirmations.sol

@@ -294,15 +294,15 @@ contract PreConfCommitmentStore is Ownable {
         );
         // This helps in avoiding stack too deep
         {
-            bytes32 preConfHash = getPreConfHash(
+            bytes32 commitmentDigest = getPreConfHash(
                 txnHash,
                 bid,
                 blockNumber,
                 bHash,
                 _bytesToHexString(bidSignature)
             );
 
-            address commiterAddress = preConfHash.recover(commitmentSignature);
+            address commiterAddress = commitmentDigest.recover(commitmentSignature);
 
             require(stake > (10 * bid), "Stake too low");
 
@@ -314,14 +314,13 @@ contract PreConfCommitmentStore is Ownable {
                 blockNumber,
                 bHash,
                 txnHash,
-                preConfHash,
+                commitmentDigest,
                 bidSignature,
                 commitmentSignature
             );
 
             commitmentIndex = getCommitmentIndex(newCommitment);
 
-
             // Store commitment
             commitments[commitmentIndex] = newCommitment;
 
@@ -331,6 +330,10 @@ contract PreConfCommitmentStore is Ownable {
 
             commitmentCount++;
             commitmentsCount[commiterAddress] += 1;
+
+            // Check if Bid has bid-amt stored
+            bidderRegistry.LockBidFunds(commitmentDigest, bid, bidderAddress);
+
         }
 
         return commitmentIndex;
@@ -404,6 +407,8 @@ contract PreConfCommitmentStore is Ownable {
             commitment.commiter,
             payable(commitment.bidder)
         );
+
+        bidderRegistry.unlockFunds(commitment.commitmentHash);
     }
 
     /**
@@ -422,8 +427,7 @@ contract PreConfCommitmentStore is Ownable {
         commitmentsCount[commitment.commiter] -= 1;
 
         bidderRegistry.retrieveFunds(
-            commitment.bidder,
-            commitment.bid,
+            commitment.commitmentHash,
             payable(commitment.commiter)
         );
     }

contracts/interfaces/IBidderRegistry.sol

@@ -12,13 +12,29 @@ interface IBidderRegistry {
         string commitmentSignature;
     }
 
+
+    struct BidState {
+        address bidder;
+        uint64 bidAmt;
+        State state;
+    }
+
+    enum State {
+        Undefined,
+        PreConfirmed,
+        Withdrawn
+    }
+
     function prepay() external payable;
 
+    function LockBidFunds(bytes32 commitmentDigest, uint64 bid, address bidder) external;
+
     function getAllowance(address bidder) external view returns (uint256);
 
     function retrieveFunds(
-        address bidder,
-        uint256 amt,
+        bytes32 commitmentDigest,
         address payable provider
     ) external;
+
+    function unlockFunds(bytes32 bidID) external;
 }

scripts/DeployScripts.s.sol

@@ -63,7 +63,7 @@ contract DeployScript is Script, Create2Deployer {
         bidderRegistry.setPreconfirmationsContract(address(preConfCommitmentStore));
         console.log("BidderRegistry updated with PreConfCommitmentStore address:", address(preConfCommitmentStore));
 
-        Oracle oracle = new Oracle{salt: salt}(address(preConfCommitmentStore), nextRequestedBlockNumber, msg.sender);
+        Oracle oracle = new Oracle{salt: salt}(address(preConfCommitmentStore), address(bidderRegistry), nextRequestedBlockNumber, msg.sender);
         console.log("Oracle deployed to:", address(oracle));
 
         preConfCommitmentStore.updateOracle(address(oracle));

test/BidderRegistryTest.sol

@@ -129,12 +129,13 @@ contract BidderRegistryTest is Test {
     }
 
     function test_shouldRetrieveFunds() public {
+        bytes32 bidID = keccak256("1234");
         bidderRegistry.setPreconfirmationsContract(address(this));
         vm.prank(bidder);
         bidderRegistry.prepay{value: 2 ether}();
         address provider = vm.addr(4);
-
-        bidderRegistry.retrieveFunds(bidder, 1 ether, payable(provider));
+        bidderRegistry.LockBidFunds(bidID, 1 ether, bidder);
+        bidderRegistry.retrieveFunds(bidID, payable(provider));
         uint256 providerAmount = bidderRegistry.providerAmount(provider);
         uint256 feeRecipientAmount = bidderRegistry.feeRecipientAmount();
 
@@ -154,8 +155,9 @@ contract BidderRegistryTest is Test {
         vm.prank(bidder);
         bidderRegistry.prepay{value: 2 ether}();
         address provider = vm.addr(4);
-
-        bidderRegistry.retrieveFunds(bidder, 1 ether, payable(provider));
+        bytes32 bidID = keccak256("1234");
+        bidderRegistry.LockBidFunds(bidID, 1 ether, bidder);
+        bidderRegistry.retrieveFunds(bidID, payable(provider));
 
         uint256 feerecipientValueAfter = bidderRegistry.feeRecipientAmount();
         uint256 providerAmount = bidderRegistry.providerAmount(provider);
@@ -171,7 +173,9 @@ contract BidderRegistryTest is Test {
         bidderRegistry.prepay{value: 2 ether}();
         address provider = vm.addr(4);
         vm.expectRevert(bytes(""));
-        bidderRegistry.retrieveFunds(bidder, 1 ether, payable(provider));
+        bytes32 bidID = keccak256("1234");
+        bidderRegistry.LockBidFunds(bidID, 1 ether, bidder);
+        bidderRegistry.retrieveFunds(bidID, payable(provider));
     }
 
     function testFail_shouldRetrieveFundsGreaterThanStake() public {
@@ -184,8 +188,9 @@ contract BidderRegistryTest is Test {
         address provider = vm.addr(4);
         vm.expectRevert(bytes(""));
         vm.prank(address(this));
-
-        bidderRegistry.retrieveFunds(bidder, 3 ether, payable(provider));
+        bytes32 bidID = keccak256("1234");
+        bidderRegistry.LockBidFunds(bidID, 3 ether, bidder);
+        bidderRegistry.retrieveFunds(bidID, payable(provider));
     }
 
     function test_withdrawFeeRecipientAmount() public {
@@ -194,7 +199,9 @@ contract BidderRegistryTest is Test {
         bidderRegistry.prepay{value: 2 ether}();
         address provider = vm.addr(4);
         uint256 balanceBefore = feeRecipient.balance;
-        bidderRegistry.retrieveFunds(bidder, 1 ether, payable(provider));
+        bytes32 bidID = keccak256("1234");
+        bidderRegistry.LockBidFunds(bidID, 1 ether, bidder);
+        bidderRegistry.retrieveFunds(bidID, payable(provider));
         bidderRegistry.withdrawFeeRecipientAmount();
         uint256 balanceAfter = feeRecipient.balance;
         assertEq(balanceAfter - balanceBefore, 100000000000000000);
@@ -213,7 +220,9 @@ contract BidderRegistryTest is Test {
         bidderRegistry.prepay{value: 5 ether}();
         address provider = vm.addr(4);
         uint256 balanceBefore = address(provider).balance;
-        bidderRegistry.retrieveFunds(bidder, 2 ether, payable(provider));
+        bytes32 bidID = keccak256("1234");
+        bidderRegistry.LockBidFunds(bidID, 2 ether, bidder);
+        bidderRegistry.retrieveFunds(bidID, payable(provider));
         bidderRegistry.withdrawProviderAmount(payable(provider));
         uint256 balanceAfter = address(provider).balance;
         assertEq(balanceAfter - balanceBefore, 1800000000000000000);
@@ -260,7 +269,9 @@ contract BidderRegistryTest is Test {
         vm.prank(bidder);
         bidderRegistry.prepay{value: 5 ether}();
         uint256 balanceBefore = address(bidder).balance;
-        bidderRegistry.retrieveFunds(bidder, 2 ether, payable(provider));
+        bytes32 bidID = keccak256("1234");
+        bidderRegistry.LockBidFunds(bidID, 2 ether, bidder);
+        bidderRegistry.retrieveFunds(bidID, payable(provider));
         vm.prank(bidderRegistry.owner());
         bidderRegistry.withdrawProtocolFee(payable(address(bidder)));
         uint256 balanceAfter = address(bidder).balance;

test/OracleTest.sol

@@ -54,7 +54,7 @@ contract OracleTest is Test {
         vm.startPrank(ownerInstance);
         bidderRegistry.prepay{value: 2 ether}();
 
-        oracle = new Oracle(address(preConfCommitmentStore), 2, ownerInstance);
+        oracle = new Oracle(address(preConfCommitmentStore), address(bidderRegistry), 2, ownerInstance);
         oracle.addBuilderAddress("mev builder", ownerInstance);
         vm.stopPrank();