feat(consensus): Add consensus check for block sequencing #231
Conversation
|
|
||
| /// The optional authorized signer for system contract consensus. | ||
| #[arg(long = "consensus.authorized-signer", value_name = "ADDRESS")] | ||
| pub authorized_signer: Option<Address>, |
There was a problem hiding this comment.
Would it be purer to only fetch authorized_signer based on the system contract's slot? But let me ask the rationale of adding this config first.
There was a problem hiding this comment.
This is primarily for testing purposes, such that we can run unit tests and set the initial signer without an L1 provider. I agree with the general sentiment. How about we put this behind a test-utils feature flag?
Not needed, you just have to be specific about how you configure the node. There are 2 ways you can configure the nodes such that L1 provider isn't needed:
|
| @@ -9,4 +9,5 @@ exec rollup-node node --chain dev --datadir=/l2reth --metrics=0.0.0.0:6060 --net | |||
| --txpool.pending-max-count=1000 \ | |||
| --builder.gaslimit=20000000 \ | |||
| --rpc.max-connections=5000 \ | |||
| --trusted-peers enode://3983278a7cab48862d9ab3187278edf376a0736a7deb55472a5650592f6922ce626a1ea7d74b77b9a679694b343f5e93ea97d5d60a9db4e4b51bb0c23a36d01b@rollup-node-sequencer:30303 | |||
| --trusted-peers enode://3983278a7cab48862d9ab3187278edf376a0736a7deb55472a5650592f6922ce626a1ea7d74b77b9a679694b343f5e93ea97d5d60a9db4e4b51bb0c23a36d01b@rollup-node-sequencer:30303 \ | |||
| --consensus.algorithm=noop | |||
There was a problem hiding this comment.
@colinlyguo I am setting the consensus algorithm to noop for this PR which is the same behaviour as we previously had from use of the --test flag. However, I think it would be good to migrate to system-contract consensus once we have anvil introduced from your open PR.
Overview
This PR introduces a
should_sequence_blockon theConsensustrait such that the manager can asses if the sequencer is authorized to sign a new block for a specific slot.