Bid update to include Shutter specific fields#5
Conversation
blockchainluffy
commented
Sep 9, 2025
blockchainluffy
commented
- This PR introduces ShutterisedBidOption, containing identityPrefix field for shutter encrypted transaction.
- It also allows for encrypted transactions to be sent via rawTransactions field of the Bid, not checking it for validity of the transaction.
| case *bidderapiv1.BidOption_PositionConstraint: | ||
| c := bOpt.GetPositionConstraint() | ||
| if c == nil { | ||
| continue |
There was a problem hiding this comment.
this looks strange, would suggest to revert to a tagless switch with bOpt.GetPositionConstraint() != nil or bOptGetShutterisedBidOption() != nil respectively
| opt := &providerapiv1.BidOption{ | ||
| Opt: &providerapiv1.BidOption_ShutterisedBidOption{ | ||
| ShutterisedBidOption: &providerapiv1.ShutterisedBidOption{ | ||
| IdentityPrefix: c.GetIdentityPrefix(), |
There was a problem hiding this comment.
Isn't EncryptedTransaction missing here?
| }; | ||
|
|
||
| message ShutterisedBidOption { | ||
| string identity_prefix = 1 [(grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { |
There was a problem hiding this comment.
same as above I guess: We added the encrypted tx to the bid option in the bidder API, don't we need it in the provider API as well?
| switch option.GetOpt().(type) { | ||
| case *bidderapiv1.BidOption_ShutterisedBidOption: | ||
| c := option.GetShutterisedBidOption() | ||
| if c == nil || c.GetIdentityPrefix() == "" || c.GetEncryptedTx() == "" { |
There was a problem hiding this comment.
I'm unsure about the validation of bids, it seems we hardly do any at the moment. Is this already handled somewhere else? In line 148 of this file there's a validation function, would it make sense to add something there? Concretely, we don't check that the identity prefix has the right size (unless it's checked automatically somehow through the protobuf definitions, but then I don't see why we check for non-emptyness). Also, the mapping from tx hash to bid option seems brittle, is it guaranteed that the lengths are equal (including, is it possible to have 0 or 2 or more bid options per trasaction)?
There was a problem hiding this comment.
makes sense, added checks for identity prefix length and removed constraint on bidOption length
Co-authored-by: jannikluhn <jannik.luhn@posteo.de>
Co-authored-by: jannikluhn <jannik.luhn@posteo.de>
Co-authored-by: jannikluhn <jannik.luhn@posteo.de>
Co-authored-by: jannikluhn <jannik.luhn@posteo.de>
Co-authored-by: jannikluhn <jannik.luhn@posteo.de>
Co-authored-by: jannikluhn <jannik.luhn@posteo.de>
Co-authored-by: jannikluhn <jannik.luhn@posteo.de>
| properties: | ||
| identityPrefix: | ||
| type: string | ||
| description: Hex string encoding of the identity prefix of the transaction that the bidder wants to include in the block. |
There was a problem hiding this comment.
I think we should standardize all hex-encoded fields to consistently include the 0x prefix. This follows Ethereum conventions and the approach we use in the Shutter API, and it also makes it immediately clear from the string representation which encoding we’re dealing with.
There was a problem hiding this comment.
The standard followed in mev-commit repo, is to not include 0x prefix to the hex encoded strings, which is also followed here!
3 participants
