WIP: Finish Relay Verification #93
Conversation
PatStiles
changed the title
There was a problem hiding this comment.
nice work!
I left some comments / suggestions but generally this is moving in the right direction :)
If we want to work on all of the validations in this set of files, I think it bears having some more direction / design work which I'd need to go do.... I'd suggest moving that to another issue where we can work on each of the validations
For now, I'd just keep the update to the using the genesis_validators_root in validate_signed_block -- feel free to make a new PR w/ just those changes or remove the others from this one
and in the meantime I can circle around to adding some more of my thinking for handling the rest of the validations
mev-relay-rs/src/relay.rs
Outdated
| @@ -62,22 +74,30 @@ fn validate_signed_block( | |||
| signed_block: &mut SignedBlindedBeaconBlock, | |||
| public_key: &BlsPublicKey, | |||
| local_payload: &mut ExecutionPayload, | |||
| current_slot: Slot, | |||
| genesis_validators_root: Root, | |||
|
|
|||
mev-relay-rs/src/relay.rs
Outdated
| @@ -132,6 +157,10 @@ impl Relay { | |||
| self.load_full_validator_set().await; | |||
| } | |||
|
|
|||
| pub fn get_current_slot(&self) -> u64 { | |||
| return self.state.lock().current_slot; | |||
There was a problem hiding this comment.
| return self.state.lock().current_slot; | |
| self.state.lock().current_slot |
mev-rs/src/types/mod.rs
Outdated
| @@ -239,6 +240,13 @@ impl ExecutionPayload { | |||
| Self::Capella(payload) => payload.gas_limit, | |||
| } | |||
| } | |||
|
|
|||
| pub fn fee_recipient(&self) -> &ByteVector<20> { | |||
There was a problem hiding this comment.
| pub fn fee_recipient(&self) -> &ByteVector<20> { | |
| pub fn fee_recipient(&self) -> &ExecutionAddress { |
mev-relay-rs/src/relay.rs
Outdated
| builder: NullBuilder, | ||
| validator_registry, | ||
| context, | ||
| //TODO: Can we rely on the future to poll and update this upon initialization instead of initializing it? |
mev-relay-rs/src/relay.rs
Outdated
| if execution_payload.fee_recipient() != &preferences.fee_recipient { | ||
| return Err( | ||
| Error::UnknownFeeRecipient(preferences.public_key.to_owned(), | ||
| ExecutionAddress::try_from(execution_payload.fee_recipient().to_owned()).unwrap()) | ||
| ) | ||
| } |
There was a problem hiding this comment.
the general pattern we are seeing is that builders will include a payment transaction but set themselves as the fee_recipient of the block
so as written, this validation would fail for most(all?) mainnet blocks
I'd suggest just leaving it out for now...
mev-relay-rs/src/relay.rs
Outdated
| @@ -132,6 +157,10 @@ impl Relay { | |||
| self.load_full_validator_set().await; | |||
| } | |||
|
|
|||
| pub fn get_current_slot(&self) -> u64 { | |||
There was a problem hiding this comment.
| pub fn get_current_slot(&self) -> u64 { | |
| fn get_current_slot(&self) -> Slot { |
mev-relay-rs/src/relay.rs
Outdated
| } | ||
|
|
||
| // verify proposer_index is correct | ||
| // -> Isn't this checked by successfully returning the publickey? |
There was a problem hiding this comment.
so for a given valid bid, it will sit on a particular fork of the chain, and it is possible for the proposer index to differ across the same slot in this case
so the intention here was to get our local view of the proposer and verify this bid was for the right validator
mev-relay-rs/src/relay.rs
Outdated
| if slot < current_slot || slot > current_slot + PROPOSAL_TOLERANCE_DELAY { | ||
| return Err(Error::UnknownBlock); | ||
| } |
There was a problem hiding this comment.
if a bid comes in at the end of the slot we could get to a case where the bid races our update of time
if anything I'd rather convert the current slot to a timestamp and allow for some propagation tolerance, idk 500ms, to enforce any time deadlines here
mev-relay-rs/src/relay.rs
Outdated
| @@ -160,7 +190,8 @@ impl BlindedBlockProvider for Relay { | |||
| } | |||
|
|
|||
| async fn fetch_best_bid(&self, bid_request: &BidRequest) -> Result<SignedBuilderBid, Error> { | |||
| validate_bid_request(bid_request)?; | |||
| let is_registered_public_key = self.validator_registry.get_validator_index(&bid_request.public_key); | |||
There was a problem hiding this comment.
if you just want to pass in a boolean here, I'd suggest this:
| let is_registered_public_key = self.validator_registry.get_validator_index(&bid_request.public_key); | |
| let is_registered_public_key = self.validator_registry.get_validator_index(&bid_request.public_key).is_some(); |
mev-relay-rs/src/relay.rs
Outdated
| if bid_request.slot < current_slot || bid_request.slot > current_slot + PROPOSAL_TOLERANCE_DELAY { | ||
| return Err(Error::UnknownBid); | ||
| } |
There was a problem hiding this comment.
same thing here on the timing...
PatStiles
force-pushed
the
finish_relay
branch
2 times, most recently
from
670ecf6
to
f346e1c
Compare
resolves #45