Skip to content

Commit

Permalink
new global cli flag ledgerLiveMode, and new config derivationPath (#488)
Browse files Browse the repository at this point in the history
### Description

allowing users to set a default base derivation path allows users to
continue to use celo path as default but for new user of cli to use eth
path.

ledger live option makes it easy to for users to use same accounts on
ledger live as they use in cli. otherwise ledger live would be the only
place to access them which was a bit scary at the worst and inconvenient
at best.

#### Other changes

breaking. refactor newLedgerWalletWithSetup to take an options object as
we are passing about 5 params.
passing in array of changeIndexes to iterate over while searching for
addresses on ledger

### QA

try out setting the config, then calling account:new, then using
useLedger flag, also try --useLedger with --ledgerLiveMode.

### Related issues 

fixes #448

<!-- start pr-codex -->

---

## PR-Codex overview
This PR introduces enhancements to the handling of derivation paths in
the Celo CLI and wallet functionalities, particularly for Ledger
devices. It adds new flags, improves documentation, and modifies related
interfaces and commands.

### Detailed summary
- Added `--ledgerLiveMode` flag to iterate over derivation paths for
Ledger.
- Introduced `DerivationPath` type in
`packages/sdk/base/src/account.ts`.
- Updated `config:set` command to support `--derivationPath` option.
- Enhanced documentation for derivation paths and CLI commands.
- Modified `newLedgerWalletWithSetup` and `LedgerWallet` interfaces to
accommodate new options.
- Added tests for `--derivationPath` handling in `Set` command.
- Updated multiple CLI commands to include `--ledgerLiveMode` and
`--derivationPath`.

> The following files were skipped due to too many changes:
`packages/sdk/wallets/wallet-ledger/src/ledger-wallet.ts`,
`packages/cli/src/base-l2.test.ts`,
`packages/sdk/wallets/wallet-ledger/src/ledger-wallet.test.ts`,
`docs/sdk/wallet-ledger/classes/ledger_wallet.LedgerWallet.md`,
`docs/command-line-interface/validator.md`,
`docs/command-line-interface/releasecelo.md`,
`docs/command-line-interface/governance.md`,
`docs/command-line-interface/account.md`

> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your
question}`

<!-- end pr-codex -->
  • Loading branch information
aaronmgdr authored Dec 17, 2024
1 parent 2c37ec4 commit 07c4c78
Show file tree
Hide file tree
Showing 48 changed files with 1,399 additions and 291 deletions.
5 changes: 5 additions & 0 deletions .changeset/healthy-days-reply.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@celo/celocli': minor
---

add --derivationPath to config:set for setting global deriviationPath. Usefull when using ledger devices and when generated new accounts.
6 changes: 6 additions & 0 deletions .changeset/kind-cups-reflect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@celo/wallet-ledger': minor
---

It is now possible to iterate over both the change and address indexes to look thru more bip44 paths for addresses. just pass changeIndexes into LedgerWallet.

42 changes: 42 additions & 0 deletions .changeset/nasty-feet-clean.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
---
'@celo/wallet-ledger': major
---

Interfaces for newLedgerWalletWithSetup and LedgerWallet constructor have changed.

`newLedgerWalletWithSetup` replaced positional arguments for named arguments and added changeIndexes

```diff
newLedgerWalletWithSetup(
transport: any,
- derivationPathIndexes?: number[],
- baseDerivationPath?: string,
- ledgerAddressValidation?: AddressValidation,
- isCel2?: boolean
+ options: LedgerWalletSetup
)

+ interface LedgerWalletSetup {
+ derivationPathIndexes?: number[]
+ changeIndexes?: number[]
+ baseDerivationPath?: string
+ ledgerAddressValidation?: AddressValidation
+ isCel2?: boolean
+ }

```

`new LedgerWallet` moved transport to first position and added changeIndexes in 4th


```diff
new LedgerWallet(
+ transport,
derivationPathIndexes,
baseDerivationPath,
- transport,
+ changeIndexes,
ledgerAddressValidation,
isCel2
)
```
5 changes: 5 additions & 0 deletions .changeset/neat-seahorses-admire.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@celo/celocli': minor
---

Add --ledgerLiveMode flag for use with --useLedger flag. This is useful for using same addresses on celocli as LedgerLive uses.
5 changes: 5 additions & 0 deletions .changeset/yellow-cameras-talk.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@celo/base': patch
---

New export type DerivationPath. Useful for when a value Must match first part of BIP44
1 change: 1 addition & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ jobs:
- name: Test that it prints out alfajores contracts
run: |
celocli network:info --node alfajores
celocli account:new
cli-tests-anvil:
name: CeloCli Tests (Anvil)
Expand Down
Loading

0 comments on commit 07c4c78

Please sign in to comment.