-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
feat(protocol): implement batch block auction #13836
Conversation
Co-authored-by: dantaik <dantaik@users.noreply.github.com> Co-authored-by: dave | d1onys1us <13951458+d1onys1us@users.noreply.github.com>
@@ -114,6 +123,13 @@ library TaikoData { | |||
uint96 amount; | |||
} | |||
|
|||
struct Bid { | |||
uint256 minFeePerGasInWei; |
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.
feePerGas
shall be fine
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 think we should have a Auction struct and a Bid struct
// 2 bytes32
struct Bid {
uint64 batchId;
address prover;
uint16 proofWindow; // allow bider to use a smaller window to win a bid?
uint64 deposit;
uint64 feePerGas;
}
// still 2 bytes32
struct Auction {
Bid bid;
uint64 startedAt;
}
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.
proofWindow: It is very confusing. Is it auction window or proof window ? (Based on the comment the former). If so, why would we allow bidder to set itself a smaller auction window ? I'd skip this var.
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.
Two reasons to have the proving window there:
- the proving window should be adjusted automatically after a block is verified, but we do not to want to proving window for a specific block or a batch to be dynamic, so better to keep a copy there when the auction started.
- The proving window is another parameter to the bid comparison function, this allows us to favor bids that promises with earlier proofs to win if we want.
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.
Ok, i have a config.worstCaseProofWindowInSec
by default so that after that everyone could submit a proof regardless of the auciton winner. I can put this proofWindow and use it as 'scoring' algo.
@@ -126,6 +142,7 @@ library TaikoData { | |||
) forkChoiceIds; | |||
mapping(address account => uint256 balance) taikoTokenBalances; | |||
mapping(bytes32 txListHash => TxListInfo) txListInfo; | |||
mapping(uint256 batchStartBlockId => Bid bid) bids; |
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.
Use a batch ID here is better I think, batchId = (blockId - 1) / batch_size
* being paid as a reward for proving the block. | ||
*/ | ||
|
||
function bidForBatch(uint256 startBlockId, uint256 minFeePerGasInWei) |
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.
function bidForBlocks(Bid calldata bid) {
}
uint256 batchStartBlockId, | ||
uint256 minFeePerGasInWei | ||
) internal { | ||
// If it verified already -> Revert, otherwise it would be possible |
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.
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.
Block proposal and batch auction can be entirely independent: we can have a million blocks proposed and we are still auctioning the first 100K blocks, or we can have more block batch auctioned but a few blocks proposed. Decoupling these two will make things easier.
Since proofs cannot be submitted without its batch auction complete. When someone bid for a batch, we do not need to check if lastVerifiedBlockId is smaller/larger than the first block in the batch. We just need to check if the auction has started/ended.
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.
Please see this comment from Brecht:
#13813 (comment)
The auctions can be for block ids %n, and so also n the number of auctions that need to be run for a sequential range of blocks, that way we make the system more parallelizable because now the prover doesn't have to be able to prove all sequential blocks himself, the work can be shared between up to n provers.
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.
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.
Ok after discussion (Discord) i'll proceed with Daniel's solution.
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.
Lets expect provers to have their own ways of distributing blocks of a batch to its workers. They may have their own auction offchain.
// If there is an existing bid already | ||
// we compare this bid to the existing one first, to see if its a | ||
// lower fee accepted. | ||
if (currentBid.bidder != address(0)) { |
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.
Always use a ring buffer please, this check is then incorrect -- we need to check if batchId matches, bidder (call it prover please) will not be reset to address(0).
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.
When we use Brecht's proposal, there is no need the same ring buffer, because a batchId (aka batchStartBlockId
) is already kind of 'ring'.
See comment above here.
UPDATE: Agreed in Discord we continue using Daniel's consecutive batch proposal.
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 think we cannot use ring buffer if we are allowing auctioning for future blocks independently of the amount of proposed blocks. So in you example:
Block proposal and batch auction can be entirely independent: we can have a million blocks proposed and we are still auctioning the first 100K blocks, or we can have more block batch auctioned but a few blocks proposed. Decoupling these two will make things easier.
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.
So if we have 10.000 block proposed, but Alice wants to bid for the proving rights of the 100.000th block, what kind of ring buffer we could use ? We simply have this below , and that's it.
funciton blockIdToBatch(uint blockId) {
return (blockId - 1)/batch_size;
}
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.
They are not sharing the same ring buffer, so why it is not possible?
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 get the idea of ring with batchId
s. So if anyone could bid to future proofs then there is no 'limit' of what could be the size of the ring, therefore no need to have a ring IMO.
// allow to fail in case they have a bad onTokenReceived | ||
// so they cant be outbid | ||
} | ||
emit BidDepositRefunded( |
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 don't think we need to emit this event.
revert L1_BID_NOT_ACCEPTABLE(); | ||
} else { | ||
// otherwise we have a new high bid, and can refund the previous claimer deposit | ||
try tkoToken.transfer(currentBid.bidder, currentBid.deposit) {} |
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.
We should use mint/burn here...
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.
Curious on why ? The current and prev. tokenomics just kept track this reward balance, i think it is simple than mint/burn.
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.
Fine for bids, but for reward and penalty, we should use mint/burn.
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.
After playing around with it, i prefer:
- Having a "deposit pool" (same as with current system) and keep track of via
state.taikoTokenBalances[msg.sender]
- Everything goes through it (proposing fee deducted from it, deposit placed from it, and refunded, etc.) except the burn when penalty. It is much more gas efficient and people (provers) could claim / withdraw as they wish, just like with the current system.
Would not throw away a good, working system.
|
||
// if a current bid exists, and the delay after the current bid exists | ||
// has passed, bidding is over. | ||
if (!isBiddingOpenForBlock(config, currentBid)) { |
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.
isBatchAuctionable(uint64 batchId)
?
|
||
// isBidAcceptable determines is checking if the bid is acceptable based on the defined | ||
// criteria | ||
function isBidAcceptable( |
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.
We also need to expose a public function
isBidAcceptable(Bid) {}
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.
Yes, i'd expose the external interfaces via TaikoL1 because these functions need state/config.
@adaki2004 Do you think we can get this done by the end of this week? |
I'm almost sure with testing + simulation it is a no. Excluding test + simulaiton, yes. |
@cyberhorsey @davidtaikocha Notes:
If a bid is OK formally, there is a check if better 'in-value', checked by the
There are public APIs which can be queried:
Basic workflow:
|
Would another public helper, to extend |
I might just use this calc comparison off-chain, because it is a scoring system, so depending on multiple factors.
|
So this is the implementation of the Auction mechanism. Not tested and by any means it is ready !
I'd like to first challenge the design by you, then create tests when we think it is good to go.
You can see interfaces introduced in
TaikoL1.sol
, but some desc.:bidForBatch()
: Bids for a batch with the following struct:batchId
can be determined for a block by calling:blockIdToBatchId(uint256 blockId)
Batches currently a range of 100 and this:
id(0):
1..100,id(1):
101..200, etc.isBlockProvableBy(uint256 blockId, address prover)
: It return if a block is proveable by that specified address. It can return true in 2 cases :1: address owns the winning bid (auction bid window elapsed, but proofTime window not yet),
2: an address tho does not own the winning bid, but the proof window (currently set to 2 horus) elapsed
isBidAcceptable(TaikoData.Bid calldata bid)
: A helper for the clients, to know if their wannabe bids can be accepted based on the criteria (either bc there is not yet a bid, or 90% better then the current winning bid)-
isBatchAuctionable(uint256 batchId)
: Again a helper, to see if auction is ongoing (either bc. there is no bid, or there is at least one bid but the time for auction window not yet elapsed)Open questions:
_markBlockVerified()
)Is it OK ? Shall i modify something ?