celo-org / celo-org/developer-tooling

Bridge commands duplicate signer construction and miss new signer types

Open
#794 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
44
Forks
36
PR merge metrics
No merged PRs in 30d

Description

## Summary

Three `bridge` commands bypass `BaseCommand`'s wallet construction and reimplement it, so any signer type added to `BaseCommand` silently does not work for them.

## Detail

`packages/cli/src/commands/bridge/deposit.ts`, `withdraw-prove.ts` and `withdraw-finalize.ts` each declare `async init() {}` to skip `BaseCommand.init()`, then build their own wallet client in a private `getL1WalletClient()`. That method re-implements the `--useLedger` / `--privateKey` branching already present in `BaseCommand.getWalletClient()` (`packages/cli/src/base.ts:229-308`):

```ts
// packages/cli/src/commands/bridge/deposit.ts:149-162
} else if (res.flags.privateKey) {
const account = privateKeyToAccount(ensureLeading0x(res.flags.privateKey))
if (res.flags.from && !isAddressEqual(res.flags.from, account.address)) { ... }
return createWalletClient({ account, chain: config.l1Chain, transport: http(l1RpcUrl) })
}
```

The duplication is understandable — these commands sign against **L1**, not the L2 chain `--node` points at, so they need a different chain and transport. But the *signer selection* logic is identical and has been copied rather than shared.

## Why it matters

The branching is an exhaustive `if/else if` over signer types. Any signer added to `BaseCommand` — a keystore option, a new HSM, anything — works everywhere except these three commands, with no compile-time error and no runtime warning. The user just silently gets a different (or no) signer.

## Suggested fix

Extract signer *selection* from chain/transport *binding* in `BaseCommand`, so a command can ask for "the configured signer, bound to this chain and transport". The bridge commands then pass their L1 chain and transport into the shared helper instead of re-deriving the account. That collapses three copies into one and makes new signer types work everywhere by construction.

## Context

Found while scoping a `--keystore` option (like `cast --keystore`). That work is deliberately scoped to `BaseCommand` and will **not** cover the bridge commands, so `--keystore` will not work with `bridge:deposit`, `bridge:withdraw-prove` or `bridge:withdraw-finalize` until this is addressed.

Contributor guide

Open the contributing guide

Research direction

Start with BaseCommand.getWalletClient() in packages/cli/src/base.ts:229-308, then compare the wallet construction in packages/cli/src/commands/bridge/deposit.ts, withdraw-prove.ts, and withdraw-finalize.ts. Trace how each command skips BaseCommand.init() and binds its L1 chain and transport. Done means the three commands share signer selection with BaseCommand while preserving their L1 behavior and support signer types added there.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
cli, developer-experience
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.