-
Notifications
You must be signed in to change notification settings - Fork 20
fix: double spend per block #140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
|
||
| uint256 windowAmount = maxBidPerBlock[bidder][currentWindow]; | ||
|
|
||
| // Calculate the available amount for this block |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont think we should keep track of this here. This could be too much for the contracts. Also, OpenBid happens after the fact that all encrypted commitments are done for the block. So providers need to check this when the accept bids and not when they are opening bids. We should just check in OpenBid that provider has not done something bad, for eg if he tries to OpenBid with amounts larger than max for the window, in which case I think he should be slashed as he should not have accepted in the first place.
Also we should check that lockedFunds doesnt underflow when we retrieveFunds etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR addresses the scenario where a bidder makes multiple bids per block. The check against the window is not completely accurate. For instance, if the limit for the block is 2 ETH, the bidder could place two bids of 2 ETH each (totaling 4 ETH) for one block. If the provider does not verify the balance, these bids will be accepted by the protocol, resulting in other providers receiving nothing for their pre-confirmations, as the amount has already been deducted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also we should check that lockedFunds doesnt underflow when we retrieveFunds etc.
It's done automatically by Solidity; if an overflow happens, an error will be thrown.
https://solidity-by-example.org/hacks/overflow/
Previous implementation didn't take care of the case, when bidder make several bids per block. Also fixed wrong comparison with the amount from the window