Skip to content

Commit e29b70c

Browse files
committed
fix(kmsServer): address PR #111 review — L1 block client + address validation
- resolveLatestRelease fetched the release block timestamp via cc.ethclient, but on baseContractCaller that's the L2/Base client while the release block number comes from the AppController on L1 — L1/L2 heights aren't interchangeable, so the timestamp would be wrong or the lookup would fail. Add SetAppControllerBlockClient(*ethclient.Client) (concrete, not on the interface) to supply the L1 client; resolveLatestRelease uses it for HeaderByNumber and falls back to cc.ethclient for single-chain setups. main.go wires l1Client. - main.go: validate app-controller-address with common.IsHexAddress before HexToAddress (which never errors and would silently use a wrong/zero address).
1 parent 80be587 commit e29b70c

3 files changed

Lines changed: 35 additions & 2 deletions

File tree

cmd/kmsServer/main.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,9 @@ func runKMSServer(c *cli.Context) error {
539539
// optional config: if unset, /secrets release lookups stay disabled (e.g. for a
540540
// signing-only deployment).
541541
if appControllerAddr := c.String("app-controller-address"); appControllerAddr != "" {
542+
if !common.IsHexAddress(appControllerAddr) {
543+
l.Sugar().Fatalw("Invalid app-controller-address", "value", appControllerAddr)
544+
}
542545
addr := common.HexToAddress(appControllerAddr)
543546
appCtrl, acErr := caller.NewAppControllerAdapter(addr, l1Client)
544547
if acErr != nil {
@@ -547,6 +550,10 @@ func runKMSServer(c *cli.Context) error {
547550
if acErr := baseContractCaller.SetAppController(appCtrl); acErr != nil {
548551
l.Sugar().Fatalw("Failed to set AppController", "error", acErr)
549552
}
553+
// The AppController is on L1, so release block numbers are L1 heights; give
554+
// the (L2/Base-backed) baseContractCaller the L1 client for the release
555+
// block-timestamp lookup in resolveLatestRelease.
556+
baseContractCaller.SetAppControllerBlockClient(l1Client)
550557
l.Sugar().Infow("AppController wired for /secrets release resolution", "address", addr.Hex())
551558
} else {
552559
l.Sugar().Warn("KMS_APP_CONTROLLER_ADDRESS not set — /secrets on-chain release resolution disabled")

pkg/contractCaller/caller/caller.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ type ContractCaller struct {
3636
releaseManager *IReleaseManager.IReleaseManager
3737
permissionController *IPermissionController.IPermissionController
3838
appController AppControllerInterface // AppController for EigenCompute apps (optional, nil until SetAppController)
39+
// appControllerClient is the L1 backend the AppController is bound to. The
40+
// AppController lives on L1, so its release block numbers are L1 heights — but
41+
// this ContractCaller's `ethclient` may be the L2/Base client (the /secrets
42+
// release lookup runs on baseContractCaller). When set, resolveLatestRelease
43+
// uses this client (not `ethclient`) to fetch the release block's timestamp so
44+
// it reads the correct chain. nil => fall back to `ethclient` (single-chain).
45+
appControllerClient *ethclient.Client
3946

4047
signer transactionSigner.ITransactionSigner
4148
}

pkg/contractCaller/caller/eigenCompute.go

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/ethereum/go-ethereum/accounts/abi/bind"
1212
"github.com/ethereum/go-ethereum/common"
1313
ethTypes "github.com/ethereum/go-ethereum/core/types"
14+
"github.com/ethereum/go-ethereum/ethclient"
1415
)
1516

1617
// Env represents environment variables as a map
@@ -86,6 +87,16 @@ func (cc *ContractCaller) SetAppController(appController AppControllerInterface)
8687
return nil
8788
}
8889

90+
// SetAppControllerBlockClient sets the L1 backend used to resolve the AppController's
91+
// release block timestamps. Call this alongside SetAppController when this
92+
// ContractCaller's primary `ethclient` is on a different chain than the AppController
93+
// (e.g. baseContractCaller is L2/Base but the AppController is on L1). Passing nil is a
94+
// no-op (resolveLatestRelease then falls back to `ethclient`). Not part of
95+
// IContractCaller — it's only relevant to the concrete ContractCaller wiring.
96+
func (cc *ContractCaller) SetAppControllerBlockClient(client *ethclient.Client) {
97+
cc.appControllerClient = client
98+
}
99+
89100
// getAppController returns the AppController instance
90101
func (cc *ContractCaller) getAppController() (AppControllerInterface, error) {
91102
if cc.appController == nil {
@@ -265,8 +276,16 @@ func (cc *ContractCaller) GetLatestReleaseAsRelease(ctx context.Context, appID s
265276
return nil, err
266277
}
267278

268-
// Fetch the block header to get the authoritative release timestamp
269-
header, err := cc.ethclient.HeaderByNumber(ctx, new(big.Int).SetUint64(r.BlockNumber))
279+
// Fetch the block header to get the authoritative release timestamp. r.BlockNumber
280+
// is an AppController (L1) block height, so read it from the L1 client when one was
281+
// configured via SetAppControllerBlockClient — cc.ethclient may be the L2/Base
282+
// client (the /secrets release lookup runs on baseContractCaller), and L1/L2 block
283+
// numbers are not interchangeable. Fall back to cc.ethclient for single-chain setups.
284+
blockClient := cc.ethclient
285+
if cc.appControllerClient != nil {
286+
blockClient = cc.appControllerClient
287+
}
288+
header, err := blockClient.HeaderByNumber(ctx, new(big.Int).SetUint64(r.BlockNumber))
270289
if err != nil {
271290
return nil, fmt.Errorf("failed to fetch block header for timestamp: %w", err)
272291
}

0 commit comments

Comments
 (0)