Skip to content

Conversation

@colinlyguo
Copy link
Contributor

Overview

This PR introduces signing key configuration for the sequencer by adding CLI support for specifying a private key file. It adds a new SignerArgs struct with the --signer.key-file parameter and implements conditional validation to ensure the key file is required when the sequencer is enabled (and test mode is disabled).

closes #146.

@colinlyguo colinlyguo requested a review from frisitano June 6, 2025 13:32
Copy link
Collaborator

@frisitano frisitano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some minor suggestions inline around using hex-encoded private keys and migrating tests to signer crate.

I think we should introduce an integration test similar to the one below for the key file singer you've introduced in this PR.

#[tokio::test]
async fn test_signer_local() {
reth_tracing::init_test_tracing();
let signer = PrivateKeySigner::random();
let mut handle = Signer::spawn(Box::new(signer.clone()));
// Test sending a request
let block = ScrollBlock::default();
handle.sign_block(block.clone()).unwrap();
// Test receiving an event
let event = handle.next().await.unwrap();
let (event_block, signature) = match event {
SignerEvent::SignedBlock { block, signature } => (block, signature),
};
let hash = sig_encode_hash(&event_block);
let recovered_address = signature.recover_address_from_prehash(&hash).unwrap();
assert_eq!(event_block, block);
assert_eq!(recovered_address, signer.address());
}

Copy link
Collaborator

@frisitano frisitano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, I left a few minor nits inline.

@colinlyguo colinlyguo requested a review from frisitano June 10, 2025 19:08
frisitano
frisitano previously approved these changes Jun 11, 2025
Copy link
Collaborator

@frisitano frisitano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a minor nit inline. LGTM

Copy link
Collaborator

@frisitano frisitano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@colinlyguo colinlyguo merged commit fcf3b96 into main Jun 11, 2025
12 checks passed
@colinlyguo colinlyguo deleted the feat-add-private-key-file-configuration-for-sequencer-signing branch June 12, 2025 15:30
@colinlyguo
Copy link
Contributor Author

LGTM

Thanks! And merged this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Signer] Signing keys configuration

3 participants