-
Notifications
You must be signed in to change notification settings - Fork 4
feat(e2e tests): invalid signature penalizes peer #243
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
base: main
Are you sure you want to change the base?
Conversation
…d-block-penalizes-peer
…d-block-penalizes-peer
…d-signature-penalizes-peer
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.
One small nit otherwise lgtm!
// Use the signer configured by SignerArgs | ||
let chain_id = chain_spec.chain().id(); | ||
self.signer_args.signer(chain_id).await?.map(rollup_node_signer::Signer::spawn) | ||
None |
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 probably add a log with warn level here just to notify that the node is running without signer.
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.
Then this would print for every node start, even if the node is not configured as a sequencer.
Maybe I'm not understanding the previous behavior correctly but I think it is the same. map(rollup_node_signer::Signer::spawn)
should only get called when self.signer_args.signer(chain_id).await?
is Some(...)
. If it's None
then signer will become None
.
The if now has the same behavior except that imo it's a bit easier to understand and it changes the priority of the branches so that we can also manually configure a signer in a test
This PR addresses the following item of #141: