shrink txmgr seam, delete SignerFactoryFromConfig and test file - #520
Conversation
| return fmt.Errorf("tx manager does not implement ChainSigner") | ||
| tcfg := cfg.TxMgrConfig | ||
|
|
||
| // Mirror the txmgr's backwards-compatible HD-path resolution. |
There was a problem hiding this comment.
As we are using the KMS to sign payloads sent to the BatchAuthenticator contract do we need this?
There was a problem hiding this comment.
We use it only for the mnemonic/private-key fallback paths. When the KMS signer is enabled the hdPath is ignored. But the factory still needs it for the other two branches, and it must resolve the path identically to txmgr or a mnemonic would derive a different key.
SignerFactoryFromConfig had no callers because NewConfig had been moved onto the fork's ChainSignerFactoryFromConfig to populate Config.ChainSigner. Not only don't we want to delete upstream functions, we also want to remove the modification of the upstream NewConfig in a later commit.
NewConfig was switched onto the fork's ChainSignerFactoryFromConfig so that Config.ChainSigner could be populated for SimpleTxManager.Sign. Both of those are gone now, and txmgr only ever uses SignTransaction, which upstream's SignerFactory already provides. Switching back makes op-service/txmgr free of Espresso entirely, and gives SignerFactoryFromConfig its caller back - it only looked like a redundant copy of ChainSignerFactoryFromConfig because txmgr had been moved off it. ChainSignerFactoryFromConfig stays: the batcher needs Sign over an arbitrary hash for the Espresso transaction envelope, which a SignerFn cannot provide. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
initChainSigner derives its own signer from cfg.TxMgrConfig, mirroring the HD-path resolution in txmgr.NewConfig. Nothing checked that the two agreed. If they ever diverged, Espresso batches would be signed by a key that is not the registered batcher, the streamer would drop all of them, and L1 submission would carry on. Check that both derivations resolve to the same address and fail startup if not. Move the call from initTxManager to initEspresso, next to initKeyPair, so initTxManager carries no Espresso logic and both signing identities are set up in one place. Also note on ChainSignerFactoryFromConfig that upstream's SignerFactoryFromConfig is not redundant with it: SignerFactory yields a SignerFn, which cannot sign an arbitrary hash. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
@jjeangal - Instead of commenting I directly pushed three commits. Please let me know if you prefer collaborating differently generally. The Related: note that prompted this PR said That was a symptom, not redundancy. Unfortunately "Exactly one factory" also isn't reachable: |
3 participants
Closes #505.
The batcher now builds its Espresso
ChainSigneritself, so txmgr no longer carries any Espresso-specific signing API. This keepsop-service/txmgr's public surface identical to upstream, which matters for our rebases.