feat(v1): Vote Extension Auction #106
Conversation
proto/pob/abci/v1/auction.proto
Outdated
| // VoteExtensionInfo wraps all vote extension data into a registry which allows applications to define | ||
| // multiple different vote extensions that can be applied to state. | ||
| message VoteExtensionInfo { | ||
| // registry is the data of the vote extension. | ||
| map<string, bytes> registry = 2; | ||
| } |
There was a problem hiding this comment.
I'm digging this idea of generalized VoteExtension handling, but I still think POB isn't where something like this belongs. It belongs in the SDK IMO. We could use it if/when it's included (feel free to open an issue and PR!)
But in the meantime, I think we should use a more concrete and domain-specific type instead.
abci/auction.go
Outdated
| // the highest bidding valid transaction along with all the bundled transactions. | ||
| func (h *ProposalHandler) BuildTOB(ctx sdk.Context, voteExtensionInfo abci.ExtendedCommitInfo, maxBytes int64) TopOfBlock { | ||
| // Get the bid transactions from the vote extensions. | ||
| bidTxs := h.GetBidsFromVoteExtensions(voteExtensionInfo.Votes) |
There was a problem hiding this comment.
| bidTxs := h.GetBidsFromVoteExtensions(voteExtensionInfo.Votes) | |
| sortedBidTxs := h.GetBidsFromVoteExtensions(voteExtensionInfo.Votes) |
abci/auction.go
Outdated
|
|
||
| // VerifyTOB verifies that the set of vote extensions used in prepare proposal deterministically | ||
| // produce the same top of block proposal. | ||
| func (h *ProposalHandler) VerifyTOB(ctx sdk.Context, proposal [][]byte) (*AuctionInfo, error) { |
There was a problem hiding this comment.
| func (h *ProposalHandler) VerifyTOB(ctx sdk.Context, proposal [][]byte) (*AuctionInfo, error) { | |
| func (h *ProposalHandler) VerifyTOB(ctx sdk.Context, proposalTxs [][]byte) (*AuctionInfo, error) { |
abci/auction.go
Outdated
| auctionInfo := AuctionInfo{} | ||
| if err := auctionInfo.Unmarshal(proposal[AuctionInfoIndex]); err != nil { |
There was a problem hiding this comment.
auctionInfo doesn't need to be a pointer?
abci/auction.go
Outdated
| return nil, fmt.Errorf("expected top of block txs does not match top of block proposal") | ||
| } | ||
|
|
||
| return &auctionInfo, nil |
There was a problem hiding this comment.
Why do we need to return a pointer? Maybe just have the type creation be a pointer instead of taking its address here?
abci/proposals.go
Outdated
| tx, err := h.ProcessProposalVerifyTx(ctx, txBz) | ||
| if err != nil { | ||
| return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT} | ||
|
|
||
| // If the transaction is nil, it was unable to be decoded and thus is invalid. | ||
| if tx == nil { | ||
| invalidProposal = true | ||
| continue | ||
| } | ||
|
|
||
| bidInfo, err := h.mempool.GetAuctionBidInfo(tx) | ||
| if err != nil { | ||
| return abci.ResponseProcessProposal{Status: abci.ResponseProcessProposal_REJECT} | ||
| invalidProposal = true | ||
| txsToRemove[tx] = struct{}{} | ||
| continue |
There was a problem hiding this comment.
I would condense this:
if tx == nil || err != nil {
invalidProposal = true
if tx != nil {
xsToRemove[tx] = struct{}{}
}
continue
}
abci/proposals.go
Outdated
| const ( | ||
| // MinProposalSize is the minimum size of a proposal. Each proposal must contain | ||
| // at least the auction info. | ||
| MinProposalSize = 1 |
There was a problem hiding this comment.
I found myself tripping up over this variable name and its use a few times. I would maybe consider renaming this to something a bit more intuitive or descriptive.
It seems this variable is primarily used to keep a cursor or know the number of "fake" txs injected into a proposal. Perhaps NumInjectedTxs? or something like...
Overview
Hosting the top of block auction using vote extensions.