Skip to content

Commit

Permalink
Fixes bug and updates tests to accomodate fund movement with bid.
Browse files Browse the repository at this point in the history
  • Loading branch information
ckartik committed Jan 30, 2024
1 parent 4d6c2ff commit b0041f2
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 28 deletions.
25 changes: 12 additions & 13 deletions contracts/BidderRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;

Expand All @@ -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);
}

/**
Expand Down
6 changes: 6 additions & 0 deletions contracts/Oracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
9 changes: 7 additions & 2 deletions contracts/PreConfirmations.sol
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,12 @@ contract PreConfCommitmentStore is Ownable {
commitment.commiter,
payable(commitment.bidder)
);

// Return funds to bidder
bidderRegistry.retrieveFunds(
commitment.commitmentHash,
payable(commitment.bidder)
);
}

/**
Expand All @@ -425,8 +431,7 @@ contract PreConfCommitmentStore is Ownable {
commitmentsCount[commitment.commiter] -= 1;

bidderRegistry.retrieveFunds(
commitment.bidder,
commitment.bid,
commitment.commitmentHash,
payable(commitment.commiter)
);
}
Expand Down
5 changes: 2 additions & 3 deletions contracts/interfaces/IBidderRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
31 changes: 21 additions & 10 deletions test/BidderRegistryTest.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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);
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit b0041f2

Please sign in to comment.