Skip to content

Conversation

@Himess
Copy link
Contributor

@Himess Himess commented Jan 13, 2026

Closes #390

@cb-heimdall
Copy link
Collaborator

cb-heimdall commented Jan 13, 2026

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1

>,
) -> Self {
let base_builder_tx = BuilderTxBase::new(Some(signer));
let base_builder_tx = BuilderTxBase::new(Some(signer.clone()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a huge fan of the .clone()s but it's not a big deal.

Copy link
Contributor

@refcell refcell left a comment

Choose a reason for hiding this comment

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

This looks good to me, would appreciate a second pair of eyes though.

Copy link
Contributor

@refcell refcell left a comment

Choose a reason for hiding this comment

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

Remove clones

@cb-heimdall
Copy link
Collaborator

Review Error for refcell @ 2026-01-13 23:09:39 UTC
User failed mfa authentication, see go/mfa-help

Replace the custom secp256k1-based Signer implementation with Alloy's
PrivateKeySigner. This simplifies the codebase by reusing well-tested
cryptographic primitives from the alloy-signer crate.

Changes:
- Wrap PrivateKeySigner in Signer struct with cached address field
- Update sign_message to use SignerSync::sign_hash_sync
- Update sign_tx to use eyre::Result instead of secp256k1::Error
- Remove secp256k1 direct dependencies from tx_signer module
- Update test files to use .clone() where Signer is moved

Closes base#390
@Himess Himess force-pushed the refactor/replace-custom-signer-with-alloy branch from 1bd7a11 to 06fee44 Compare January 14, 2026 13:33
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.

refactor(op-rbuilder): Replace Custom Signer with Alloy's PrivateKeySigner

3 participants