feat(maru): support custom validator signers - #3700
Conversation
e9f67d5 to
35e5dc7
Compare
There was a problem hiding this comment.
Pull request overview
Adds an opt-in “custom” QBFT validator signing backend to Maru, allowing embedders to supply validator signer implementations without exposing the node’s P2P private key, while adapting async signing into Besu’s synchronous NodeKey interface and enforcing validator-identity validation against configured validator sets.
Changes:
- Introduces
ValidatorSignerConfigTOML parsing (signer-type,signer-name) and surfaces it through Maru’s config domain model. - Adds crypto adapter types to expose
Signer<Secp256k1Signature>as a BesuNodeKeyfor QBFT (including error wrapping and interruption preservation). - Wires validator
NodeKeycreation through the app factory, validates custom signer address membership in configured validator sets, and documents the new configuration.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| maru/README.md | Documents local vs custom QBFT validator signing configuration and behavior. |
| maru/crypto/src/test/kotlin/maru/crypto/ValidatorSignerTest.kt | Adds unit tests for NodeKey adaptation, digest forwarding, and error/interrupt handling. |
| maru/crypto/src/main/kotlin/maru/crypto/ValidatorSigner.kt | Implements local validator signer + Besu SecurityModule adapter and toNodeKey() helper. |
| maru/crypto/build.gradle | Adds dependencies needed for Besu plugin SecurityModule types and signer interfaces. |
| maru/consensus/src/test/kotlin/maru/consensus/qbft/FollowerBeaconBlockImporterTest.kt | Adds coverage ensuring signing failures propagate as failed futures. |
| maru/consensus/src/main/kotlin/maru/consensus/qbft/QbftValidatorFactory.kt | Switches QBFT validator identity/signing to use an injected NodeKey instead of raw private key bytes. |
| maru/consensus/src/main/kotlin/maru/consensus/blockimport/BeaconBlockImporter.kt | Wraps prevRandao/signing failures so block import returns a failed future. |
| maru/config/src/test/kotlin/maru/config/ValidatorSignerConfigTest.kt | Adds unit tests for signer config defaults/validation and TOML parsing. |
| maru/config/src/main/kotlin/maru/config/HopliteTomlFriendly.kt | Extends QBFT TOML DTO with signer fields and maps to domain config. |
| maru/config/src/main/kotlin/maru/config/Config.kt | Introduces ValidatorSignerConfig / ValidatorSignerType and attaches it to QbftConfig. |
| maru/app/src/test/kotlin/maru/app/ValidatorSignerFactoryTest.kt | Adds tests for default factory behavior and managed-signer lifecycle semantics. |
| maru/app/src/main/kotlin/maru/app/ValidatorSignerFactory.kt | Introduces ValidatorSignerFactory, ManagedValidatorSigner, and default factory behavior. |
| maru/app/src/main/kotlin/maru/app/QbftProtocolValidatorFactory.kt | Updates factory wiring to pass NodeKey into QBFT validator factory. |
| maru/app/src/main/kotlin/maru/app/MaruAppFactory.kt | Creates validator NodeKey via local/custom signer selection and threads it into MaruApp. |
| maru/app/src/main/kotlin/maru/app/MaruApp.kt | Derives validator address from the validator NodeKey, validates identity, and closes managed signer on shutdown. |
| maru/app/src/main/kotlin/maru/app/CliEntrypoint.kt | Refactors CLI entrypoint to support testable execution and factory injection. |
| maru/app/build.gradle | Exposes config + signer interfaces via api for embedders and adds Besu crypto-service deps. |
35e5dc7 to
8ee733c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (3)
maru/app/src/main/kotlin/maru/app/MaruApp.kt:330
validateValidatorIdentitythrowsIllegalArgumentException("")for unsupported fork configurations, which produces an empty error message and makes startup/debugging harder. Include the configuration type (or a clearer message) in the exception.
when (val configuration = it.configuration) {
is DifficultyAwareQbftConfig -> configuration.postTtdConfig.validatorSet
is QbftConsensusConfig -> configuration.validatorSet
else -> throw IllegalArgumentException("")
}
maru/app/src/main/kotlin/maru/app/MaruAppFactory.kt:356
- If
MaruAppconstruction fails after creatingmanagedValidatorSigner(e.g., custom signer address not present in any validator set), the signer resource is never closed. Wrap theMaruApp(...)creation in a try/catch and close the signer on failure.
return MaruApp(
config = config,
beaconGenesisConfig = beaconGenesisConfig,
clock = clock,
p2pNetwork = p2pNetwork,
maru/app/src/main/kotlin/maru/app/MaruApp.kt:226
managedValidatorSigner?.close()will be skipped ifprotocolStarter.close()throws, which can leak external signer resources on shutdown. Use atry/finallyto ensure the signer is closed even when protocol shutdown fails.
vertx.close()
protocolStarter.close()
managedValidatorSigner?.close()
// close db last, otherwise other components may fail trying to save data
beaconChain.close()
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Suppressed comments (2)
maru/app/src/main/kotlin/maru/app/MaruApp.kt:329
validateValidatorIdentitythrowsIllegalArgumentException("")for unexpected fork configurations, which results in an empty error message and makes diagnosing misconfigured genesis/forks difficult. Include the unexpected configuration type (or fork) in the exception message.
else -> throw IllegalArgumentException("")
maru/crypto/src/main/kotlin/maru/crypto/ValidatorSigner.kt:63
secpPublicKeyis stored as a class property but is only needed inside theinitblock; keeping it as a field is unnecessary and may trip unused-property checks. Make it a localvalinsideinitinstead.
private val secpPublicKey: SECPPublicKey
private val publicKey: PublicKey
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (3)
maru/crypto/build.gradle:8
maru:cryptoexposesSigner/Secp256k1Signaturein its public API (e.g.,LocalValidatorSigner), butsigner-interfacesis declared as animplementationdependency. With thejava-libraryplugin this can break compilation for downstream modules that use these public types; it should be anapidependency (or the public API should avoid referencing those types).
implementation(project(':jvm-libs:linea:core:signer-interfaces'))
maru/crypto/build.gradle:13
ValidatorSigner.ktpublicly exposes Besu'sNodeKeyviaSigner<Secp256k1Signature>.toNodeKey(), butbesu-crypto-servicesis animplementationdependency. Becausemaru:cryptoappliesjava-library, downstream modules may not seeNodeKeyon their compile classpath unless they add the dependency themselves; consider makingbesu-crypto-servicesanapidependency (or hideNodeKeybehind a Maru-owned type).
implementation("org.hyperledger.besu:besu-plugin-api")
implementation("org.hyperledger.besu.internal:besu-crypto-algorithms")
implementation("org.hyperledger.besu.internal:besu-crypto-services")
maru/app/src/main/kotlin/maru/app/MaruAppFactory.kt:488
- The warning message contains awkward/incorrect wording ("validatorSet-s"), which makes logs harder to read and search.
log.warn(
"localValidator={} isn't found in any of validatorSet-s in any of the Forks in the Genesis file!",
validator,
)
| } | ||
| } | ||
|
|
||
| internal class SignerSecurityModule( |
There was a problem hiding this comment.
We are making an adapter to adapt Signer to a third party interface, which we use instead of our interface. I believe QbftFinalStateAdapter is the only place where it's necessary. In the rest of the places we can use Signer and CloseableSigner
aa02b68 to
bae8a40
Compare
b07ccd3 to
029ebc9
Compare
9cab88c to
4d0058e
Compare
4d0058e to
2d8414d
Compare
443cf95 to
4e9336c
Compare
4e9336c to
eedb450
Compare
Signed-off-by: nadeemb53 <nadeemb53@gmail.com>
Signed-off-by: nadeemb53 <nadeemb53@gmail.com>
Signed-off-by: nadeemb53 <nadeemb53@gmail.com>
Signed-off-by: nadeemb53 <nadeemb53@gmail.com>
Signed-off-by: nadeemb53 <nadeemb53@gmail.com>
Signed-off-by: nadeemb53 <nadeemb53@gmail.com>
Signed-off-by: nadeemb53 <nadeemb53@gmail.com>
eedb450 to
e6b2752
Compare
Signed-off-by: nadeemb53 <nadeemb53@gmail.com>
c399af6 to
3f6568a
Compare
Signed-off-by: nadeemb53 <nadeemb53@gmail.com>
| } | ||
| } | ||
|
|
||
| internal class ValidatorSignerFactory( |
There was a problem hiding this comment.
Sorry for nitpicking, but I feel like it's important to nail it here.
I think we're missing the distinction between the ValidatorSignerFactory and CustomValidatorSignerFactory. At the first glance it looks like delegation, but these factories have slightly different focus. Can we reflect this difference in the naming?
There was a problem hiding this comment.
rename ValidatorSignerFactory to ValidatorSignerInitializer then?
Signed-off-by: nadeemb53 <nadeemb53@gmail.com>
Signed-off-by: nadeemb53 <nadeemb53@gmail.com>
NodeKeyused by QBFTStacked on #3666