diff --git a/contracts/BidderRegistry.sol b/contracts/BidderRegistry.sol index 4f20160..d0a9f57 100644 --- a/contracts/BidderRegistry.sol +++ b/contracts/BidderRegistry.sol @@ -47,7 +47,7 @@ contract BidderRegistry is IBidderRegistry, Ownable, ReentrancyGuard { 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. @@ -150,29 +150,24 @@ contract BidderRegistry is IBidderRegistry, Ownable, ReentrancyGuard { bidAmt: bid, state: State.PreConfirmed }); - bidderPrepaidBalances[bidder] -= bidState.bidAmt; + 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; @@ -184,7 +179,11 @@ 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); } /** diff --git a/contracts/Oracle.sol b/contracts/Oracle.sol index 1b614ad..833f78b 100644 --- a/contracts/Oracle.sol +++ b/contracts/Oracle.sol @@ -119,6 +119,12 @@ contract Oracle is Ownable { nextRequestedBlockNumber++; } + /** + */ + function returnFunds(bytes32 bidID) external onlyOwner { + // Bidder registery.returnFunds(bidID); + } + /** * @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. diff --git a/contracts/PreConfirmations.sol b/contracts/PreConfirmations.sol index 31a9c73..75ed9ca 100644 --- a/contracts/PreConfirmations.sol +++ b/contracts/PreConfirmations.sol @@ -407,6 +407,12 @@ contract PreConfCommitmentStore is Ownable { commitment.commiter, payable(commitment.bidder) ); + + // Return funds to bidder + bidderRegistry.retrieveFunds( + commitment.commitmentHash, + payable(commitment.bidder) + ); } /** @@ -425,8 +431,7 @@ contract PreConfCommitmentStore is Ownable { commitmentsCount[commitment.commiter] -= 1; bidderRegistry.retrieveFunds( - commitment.bidder, - commitment.bid, + commitment.commitmentHash, payable(commitment.commiter) ); } diff --git a/contracts/interfaces/IBidderRegistry.sol b/contracts/interfaces/IBidderRegistry.sol index 25aba7a..949f461 100644 --- a/contracts/interfaces/IBidderRegistry.sol +++ b/contracts/interfaces/IBidderRegistry.sol @@ -27,12 +27,11 @@ interface IBidderRegistry { function prepay() external payable; function IdempotentBidFundsMovement(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; } diff --git a/test/BidderRegistryTest.sol b/test/BidderRegistryTest.sol index f158c02..e1ea331 100644 --- a/test/BidderRegistryTest.sol +++ b/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.IdempotentBidFundsMovement(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.IdempotentBidFundsMovement(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.IdempotentBidFundsMovement(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.IdempotentBidFundsMovement(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.IdempotentBidFundsMovement(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.IdempotentBidFundsMovement(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.IdempotentBidFundsMovement(bidID, 2 ether, bidder); + bidderRegistry.retrieveFunds(bidID, payable(provider)); vm.prank(bidderRegistry.owner()); bidderRegistry.withdrawProtocolFee(payable(address(bidder))); uint256 balanceAfter = address(bidder).balance;