feat(signer): sign txs via KMS by Thegaram · Pull Request #1808 · scroll-tech/scroll · GitHub

Purpose or design rationale of this PR

Adds a third signer_type, AWSKMS, alongside PrivateKey and RemoteSigner.

kms_signer.go — at startup, derives the key's address from kms:GetPublicKey and refuses to run unless it matches the configured signer_address. Per tx, sends only the 32-byte signing hash to kms:Sign and converts the DER (r, s) reply into Ethereum's 65-byte form (low-s per EIP-2, recovery id found by checking which one recovers the key's own address). The private key never leaves KMS.
Why not RemoteSigner — web3signer can't sign blob txs, so commit_sender can't use it. KMS signs a hash and the tx is assembled locally, so all tx types work, blobs included.
Config — aws_kms_signer_config: key_id, optional region, required signer_address. Credentials come from the standard AWS chain (IRSA/instance role), never the config file.

Deployment checklist

Create the key: ECC_SECG_P256K1 / SIGN_VERIFY, migrate the existing private keys into KMS.
Grant the workload only kms:Sign + kms:GetPublicKey on that key.
Derive the address from get-public-key, put it in signer_address (should match known key).
Set signer_type: AWSKMS, key_id, and region in the service config.
Deploy the gas oracle sender first, then finalize, then commit — smallest blast radius first.

Summary by CodeRabbit

Release Notes

New Features
- Added AWS Key Management Service (KMS) support for transaction signing, including support for blob transactions.
Bug Fixes
- Improved signer address validation for KMS and remote-signer configurations, with clearer errors when configuration is invalid.
Documentation
- Added a comprehensive AWS KMS configuration and setup guide, including required fields and startup validation behavior.
Chores
- Version updated to v4.7.17.

📝 Walkthrough

Walkthrough

Adds AWS KMS-backed ECDSA secp256k1 transaction signing to the rollup sender, including configuration, address validation, transaction integration, tests, documentation, and dependency wiring. The version tag is updated to v4.7.17.

Changes

AWS KMS Signer Integration

Layer / File(s)	Summary
AWS KMS configuration and address resolution rollup/go.mod, rollup/internal/config/relayer.go, rollup/internal/controller/relayer/l2_relayer.go	Adds AWS KMS configuration, dependency wiring, and shared signer-address validation.
KMS signer implementation rollup/internal/controller/sender/kms_signer.go	Retrieves and validates secp256k1 public keys, derives addresses, parses DER signatures, enforces low-s, and recovers v.
TransactionSigner AWS KMS wiring rollup/internal/controller/sender/transaction_signer.go, rollup/internal/controller/sender/sender.go	Passes context through signer construction, initializes AWS KMS signing, and applies signatures to transactions.
Signer validation and transaction tests rollup/internal/controller/sender/kms_signer_test.go, rollup/internal/controller/sender/transaction_signer_test.go	Tests address validation, transaction types, low-s normalization, malformed KMS responses, and updated constructor calls.
AWS KMS sender documentation rollup/internal/controller/sender/README.md	Documents configuration, credentials, IAM permissions, key provisioning, and rotation.

Version Bump

Layer / File(s)	Summary
Version tag update common/version/version.go	Updates the version tag from v4.7.16 to v4.7.17.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant NewSender
  participant NewTransactionSigner
  participant kmsSigner
  participant AWSKMS

  NewSender->>NewTransactionSigner: context, AWS KMS config, chain ID
  NewTransactionSigner->>kmsSigner: initialize signer
  kmsSigner->>AWSKMS: GetPublicKey
  AWSKMS-->>kmsSigner: DER SPKI public key
  kmsSigner->>kmsSigner: derive and validate signer address
  NewTransactionSigner->>kmsSigner: sign transaction
  kmsSigner->>AWSKMS: Sign digest
  AWSKMS-->>kmsSigner: DER ECDSA signature
  kmsSigner-->>NewTransactionSigner: apply Ethereum signature

Possibly related PRs

scroll-tech/scroll#1809: Both PRs update the version tag in common/version/version.go.
scroll-tech/scroll#1556: Both PRs modify the package version tag, although the functional changes are unrelated.

Suggested labels: bump-version

Suggested reviewers: kchangn, georgehao, johnsonjie

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 21.43% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately captures the main change: adding AWS KMS signing support.
Description check	✅ Passed	The description covers purpose, rationale, config, and rollout steps, but omits explicit tag-versioning and breaking-change confirmations.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat-signer-kms-auth

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Codecov Report

❌ Patch coverage is 55.26316% with 68 lines in your changes missing coverage. Please review.
✅ Project coverage is 35.47%. Comparing base (788fdf0) to head (f8401ed).

Files with missing lines	Patch %	Lines
rollup/internal/controller/sender/kms_signer.go	64.00%	34 Missing and 11 partials ⚠️
rollup/internal/controller/relayer/l2_relayer.go	0.00%	13 Missing ⚠️
...p/internal/controller/sender/transaction_signer.go	23.07%	10 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1808      +/-   ##
===========================================
+ Coverage    35.44%   35.47%   +0.02%     
===========================================
  Files          262      261       -1     
  Lines        22596    22642      +46     
===========================================
+ Hits          8010     8032      +22     
- Misses       13748    13764      +16     
- Partials       838      846       +8

Flag	Coverage Δ
rollup	35.59% <55.26%> (+0.41%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

rollup/internal/controller/relayer/l2_relayer.go (1)
226-228: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Log when the committed batch’s end chunk is missing.

If GetChunkByIndex returns nil, the gauge remains at zero without any diagnostic, defeating this initialization and potentially triggering a false commit-lag alert. Emit a warning for this inconsistent database state, or fail startup if it must be treated as fatal.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rollup/internal/controller/relayer/l2_relayer.go` around lines 226 - 228,
Update the initialization flow around GetChunkByIndex and the endChunk check to
emit a warning when endChunk is nil, including enough context to identify the
missing committed batch end chunk; preserve the existing gauge update when
endChunk is present.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@rollup/internal/controller/relayer/l2_relayer.go`:
- Around line 226-228: Update the initialization flow around GetChunkByIndex and
the endChunk check to emit a warning when endChunk is nil, including enough
context to identify the missing committed batch end chunk; preserve the existing
gauge update when endChunk is present.

ℹ️ Review info ⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 27e150cf-14d6-464f-a016-a3e94905ca5c

📥 Commits

Reviewing files that changed from the base of the PR and between 3a22fc9 and 58ebdeb.

📒 Files selected for processing (2)

common/version/version.go
rollup/internal/controller/relayer/l2_relayer.go

🚧 Files skipped from review as they are similar to previous changes (1)

common/version/version.go

🧹 Nitpick comments (1)

rollup/internal/controller/sender/transaction_signer.go (1)
69-77: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Cover the NewTransactionSigner KMS branch directly.

The existing KMS test constructs TransactionSigner with kmsSigner: ks, so it bypasses this constructor path. Add a constructor-level test using an injectable KMS client/factory, or at least cover its validation/error paths, so regressions in initialization and address wiring are detected.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rollup/internal/controller/sender/transaction_signer.go` around lines 69 -
77, Add a direct test for the AWSKMSSignerType branch in NewTransactionSigner,
using an injectable KMS client or factory to exercise newKMSSigner,
initialization, and address wiring; if injection is unavailable, cover the
constructor’s validation and error paths instead. Keep the existing
TransactionSigner-based KMS test, but ensure the new test invokes
NewTransactionSigner itself and verifies successful signer creation or the
expected error.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@rollup/internal/controller/sender/transaction_signer.go`:
- Around line 69-77: Add a direct test for the AWSKMSSignerType branch in
NewTransactionSigner, using an injectable KMS client or factory to exercise
newKMSSigner, initialization, and address wiring; if injection is unavailable,
cover the constructor’s validation and error paths instead. Keep the existing
TransactionSigner-based KMS test, but ensure the new test invokes
NewTransactionSigner itself and verifies successful signer creation or the
expected error.

ℹ️ Review info ⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6a9da4be-7519-46fa-85cb-39baa850226a

📥 Commits

Reviewing files that changed from the base of the PR and between 58ebdeb and f8401ed.

📒 Files selected for processing (6)

rollup/internal/config/relayer.go
rollup/internal/controller/relayer/l2_relayer.go
rollup/internal/controller/sender/README.md
rollup/internal/controller/sender/kms_signer.go
rollup/internal/controller/sender/kms_signer_test.go
rollup/internal/controller/sender/transaction_signer.go

🚧 Files skipped from review as they are similar to previous changes (5)

rollup/internal/config/relayer.go
rollup/internal/controller/sender/README.md
rollup/internal/controller/sender/kms_signer_test.go
rollup/internal/controller/relayer/l2_relayer.go
rollup/internal/controller/sender/kms_signer.go

feat(signer): sign txs via KMS

3a22fc9

Thegaram requested review from georgehao and kchangn June 15, 2026 19:40

georgehao previously approved these changes Jun 16, 2026

View reviewed changes

Merge branch 'develop' into feat-signer-kms-auth

58ebdeb

Thegaram dismissed georgehao’s stale review via 58ebdeb July 27, 2026 10:54

coderabbitai Bot reviewed Jul 27, 2026

View reviewed changes

code review improvements

f8401ed

coderabbitai Bot reviewed Jul 27, 2026

View reviewed changes

feat(signer): sign txs via KMS by Thegaram · Pull Request #1808 · scroll-tech/scroll · GitHub

Uh oh!

feat(signer): sign txs via KMS - #1808

feat(signer): sign txs via KMS#1808
Thegaram wants to merge 3 commits into
developfrom
feat-signer-kms-auth

Thegaram commented Jun 15, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

Uh oh!

coderabbitai Bot commented Jun 15, 2026 •
edited
Loading

Uh oh!

Uh oh!

codecov-commenter commented Jun 15, 2026 •
edited
Loading

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

feat(signer): sign txs via KMS by Thegaram · Pull Request #1808 · scroll-tech/scroll · GitHub

Uh oh!

feat(signer): sign txs via KMS - #1808

feat(signer): sign txs via KMS#1808Thegaram wants to merge 3 commits intodevelopscroll-tech/scroll:developfrom feat-signer-kms-authscroll-tech/scroll:feat-signer-kms-authCopy head branch name to clipboard

Conversation

Thegaram commented Jun 15, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose or design rationale of this PR

Deployment checklist

Summary by CodeRabbit

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

❌ Failed checks (1 warning)

Uh oh!

codecov-commenter commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

feat(signer): sign txs via KMS#1808
Thegaram wants to merge 3 commits into
developfrom
feat-signer-kms-auth

Thegaram commented Jun 15, 2026 •
edited by coderabbitai Bot
Loading

coderabbitai Bot commented Jun 15, 2026 •
edited
Loading

codecov-commenter commented Jun 15, 2026 •
edited
Loading