From d988d317582daed57bf05a4c4d9d087e5e732f0d Mon Sep 17 00:00:00 2001 From: Nicolas Brugneaux Date: Tue, 12 Nov 2024 15:55:18 +0100 Subject: [PATCH] fix: ledger signature recovery value (#408) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## PR-Codex overview This PR enhances support for `celo-legacy` and modern transactions within the `ledger` wallet, including updates to transaction signing, error handling, and test cases. ### Detailed summary - Updated `package.json` for `@ledgerhq/hw-app-eth` to a specific commit. - Added `bundleDependencies` for `@ledgerhq/hw-app-eth`. - Improved `signTransaction` method in `LedgerSigner` to handle `v` values correctly. - Refactored transaction recovery logic in tests for better validation. - Enhanced error messages for legacy app version requirements. - Updated tests to validate transaction signing for both legacy and modern types. > ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}` --- .changeset/little-carpets-argue.md | 7 + .github/workflows/cron-npm-install.yml | 8 +- .../signing_utils.LegacyEncodedTx.md | 6 +- docs/sdk/wallet-base/modules/signing_utils.md | 48 +- .../classes/ledger_signer.LedgerSigner.md | 12 +- .../wallet-base/src/signing-utils.test.ts | 55 +- .../wallets/wallet-base/src/signing-utils.ts | 17 +- .../sdk/wallets/wallet-ledger/package.json | 7 +- .../wallet-ledger/src/ledger-signer.ts | 22 +- .../wallet-ledger/src/ledger-wallet.test.ts | 524 +++++++++++------- .../wallet-ledger/src/ledger-wallet.ts | 32 +- .../wallet-local/src/local-wallet.test.ts | 5 + yarn.lock | 103 +++- 13 files changed, 579 insertions(+), 267 deletions(-) create mode 100644 .changeset/little-carpets-argue.md diff --git a/.changeset/little-carpets-argue.md b/.changeset/little-carpets-argue.md new file mode 100644 index 000000000..f91b7b50f --- /dev/null +++ b/.changeset/little-carpets-argue.md @@ -0,0 +1,7 @@ +--- +'@celo/wallet-ledger': patch +'@celo/wallet-local': patch +'@celo/wallet-base': patch +--- + +Improve support for celo-legacy and modern txs within ledger diff --git a/.github/workflows/cron-npm-install.yml b/.github/workflows/cron-npm-install.yml index 56b04de68..c3e36df27 100644 --- a/.github/workflows/cron-npm-install.yml +++ b/.github/workflows/cron-npm-install.yml @@ -20,9 +20,9 @@ jobs: fail-fast: false matrix: package: - - '@celo/utils@beta' - - '@celo/contractkit@beta' - - '@celo/celocli@beta' + - '@celo/utils@beta' + - '@celo/contractkit@beta' + - '@celo/celocli@beta' steps: - name: Install @celo/celocli dependencies if: matrix.package == '@celo/celocli@beta' @@ -30,6 +30,8 @@ jobs: apt update apt install -y libusb-1.0-0-dev libudev-dev npm install node-gyp --global + git config --global url."https://".insteadOf ssh:// + - name: Installing npm package run: npm install ${{ matrix.package }} --global - name: Test celocli command diff --git a/docs/sdk/wallet-base/interfaces/signing_utils.LegacyEncodedTx.md b/docs/sdk/wallet-base/interfaces/signing_utils.LegacyEncodedTx.md index 61a1cee74..10e9e07a8 100644 --- a/docs/sdk/wallet-base/interfaces/signing_utils.LegacyEncodedTx.md +++ b/docs/sdk/wallet-base/interfaces/signing_utils.LegacyEncodedTx.md @@ -20,7 +20,7 @@ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:232](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L232) +[wallets/wallet-base/src/signing-utils.ts:240](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L240) ___ @@ -30,7 +30,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:233](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L233) +[wallets/wallet-base/src/signing-utils.ts:241](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L241) ___ @@ -40,4 +40,4 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:231](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L231) +[wallets/wallet-base/src/signing-utils.ts:239](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L239) diff --git a/docs/sdk/wallet-base/modules/signing_utils.md b/docs/sdk/wallet-base/modules/signing_utils.md index 903262371..b565a7962 100644 --- a/docs/sdk/wallet-base/modules/signing_utils.md +++ b/docs/sdk/wallet-base/modules/signing_utils.md @@ -92,7 +92,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:935](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L935) +[wallets/wallet-base/src/signing-utils.ts:946](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L946) ___ @@ -139,7 +139,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:869](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L869) +[wallets/wallet-base/src/signing-utils.ts:880](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L880) ___ @@ -159,7 +159,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:545](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L545) +[wallets/wallet-base/src/signing-utils.ts:553](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L553) ___ @@ -183,7 +183,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:368](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L368) +[wallets/wallet-base/src/signing-utils.ts:376](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L376) ___ @@ -203,7 +203,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:237](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L237) +[wallets/wallet-base/src/signing-utils.ts:245](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L245) ___ @@ -229,7 +229,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:879](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L879) +[wallets/wallet-base/src/signing-utils.ts:890](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L890) ___ @@ -255,7 +255,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:463](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L463) +[wallets/wallet-base/src/signing-utils.ts:471](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L471) ___ @@ -295,7 +295,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:535](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L535) +[wallets/wallet-base/src/signing-utils.ts:543](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L543) ___ @@ -315,7 +315,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:913](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L913) +[wallets/wallet-base/src/signing-utils.ts:924](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L924) ___ @@ -335,7 +335,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:927](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L927) +[wallets/wallet-base/src/signing-utils.ts:938](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L938) ___ @@ -355,7 +355,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:919](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L919) +[wallets/wallet-base/src/signing-utils.ts:930](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L930) ___ @@ -375,7 +375,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:901](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L901) +[wallets/wallet-base/src/signing-utils.ts:912](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L912) ___ @@ -395,7 +395,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:907](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L907) +[wallets/wallet-base/src/signing-utils.ts:918](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L918) ___ @@ -415,7 +415,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:347](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L347) +[wallets/wallet-base/src/signing-utils.ts:355](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L355) ___ @@ -435,7 +435,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:343](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L343) +[wallets/wallet-base/src/signing-utils.ts:351](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L351) ___ @@ -455,7 +455,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:339](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L339) +[wallets/wallet-base/src/signing-utils.ts:347](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L347) ___ @@ -475,7 +475,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:325](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L325) +[wallets/wallet-base/src/signing-utils.ts:333](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L333) ___ @@ -496,7 +496,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:837](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L837) +[wallets/wallet-base/src/signing-utils.ts:848](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L848) ___ @@ -516,7 +516,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:494](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L494) +[wallets/wallet-base/src/signing-utils.ts:502](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L502) ___ @@ -536,7 +536,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:126](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L126) +[wallets/wallet-base/src/signing-utils.ts:134](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L134) ___ @@ -564,7 +564,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:892](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L892) +[wallets/wallet-base/src/signing-utils.ts:903](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L903) ___ @@ -584,7 +584,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:107](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L107) +[wallets/wallet-base/src/signing-utils.ts:115](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L115) ___ @@ -606,7 +606,7 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:847](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L847) +[wallets/wallet-base/src/signing-utils.ts:858](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L858) ___ @@ -628,4 +628,4 @@ ___ #### Defined in -[wallets/wallet-base/src/signing-utils.ts:856](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L856) +[wallets/wallet-base/src/signing-utils.ts:867](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-base/src/signing-utils.ts#L867) diff --git a/docs/sdk/wallet-ledger/classes/ledger_signer.LedgerSigner.md b/docs/sdk/wallet-ledger/classes/ledger_signer.LedgerSigner.md index b66254755..6f43b9f26 100644 --- a/docs/sdk/wallet-ledger/classes/ledger_signer.LedgerSigner.md +++ b/docs/sdk/wallet-ledger/classes/ledger_signer.LedgerSigner.md @@ -72,7 +72,7 @@ Signer.computeSharedSecret #### Defined in -[wallet-ledger/src/ledger-signer.ts:197](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts#L197) +[wallet-ledger/src/ledger-signer.ts:190](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts#L190) ___ @@ -96,7 +96,7 @@ Signer.decrypt #### Defined in -[wallet-ledger/src/ledger-signer.ts:191](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts#L191) +[wallet-ledger/src/ledger-signer.ts:184](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts#L184) ___ @@ -138,19 +138,19 @@ Signer.signPersonalMessage #### Defined in -[wallet-ledger/src/ledger-signer.ts:86](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts#L86) +[wallet-ledger/src/ledger-signer.ts:79](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts#L79) ___ ### signTransaction -▸ **signTransaction**(`addToV`, `encodedTx`): `Promise`\<\{ `r`: `Buffer` ; `s`: `Buffer` ; `v`: `number` }\> +▸ **signTransaction**(`_addToV`, `encodedTx`): `Promise`\<\{ `r`: `Buffer` ; `s`: `Buffer` ; `v`: `number` }\> #### Parameters | Name | Type | | :------ | :------ | -| `addToV` | `number` | +| `_addToV` | `number` | | `encodedTx` | `RLPEncodedTx` \| `LegacyEncodedTx` | #### Returns @@ -187,4 +187,4 @@ Signer.signTypedData #### Defined in -[wallet-ledger/src/ledger-signer.ts:106](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts#L106) +[wallet-ledger/src/ledger-signer.ts:99](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts#L99) diff --git a/packages/sdk/wallets/wallet-base/src/signing-utils.test.ts b/packages/sdk/wallets/wallet-base/src/signing-utils.test.ts index e038ded62..775480387 100644 --- a/packages/sdk/wallets/wallet-base/src/signing-utils.test.ts +++ b/packages/sdk/wallets/wallet-base/src/signing-utils.test.ts @@ -545,15 +545,15 @@ describe('recoverTransaction', () => { "s": "0x1799477e0f601f554f0ee6f7ac21490602124801e9f7a99d9605249b90f03112", "to": "0x588e4b68193001e4d10928660ab4165b813717c0", "type": "cip42", - "v": 28, + "v": 27, "value": 1000000000000000000, - "yParity": 1, + "yParity": 0, }, "0x90AB065B949165c47Acac34cA9A43171bBeBb1E1", ] `) }) - test('cip42 serialized by viem', async () => { + test('cip42 serialized by viem (v=0)', async () => { const account = privateKeyToAccount(PRIVATE_KEY1) const signed = await account.signTransaction( { @@ -601,6 +601,55 @@ describe('recoverTransaction', () => { `) expect(recoverTransaction(signed)[1]).toEqual(account.address) }) + + test.only('cip42 serialized by viem (v=1)', async () => { + const account = privateKeyToAccount(PRIVATE_KEY1) + const signed = await account.signTransaction( + { + // @ts-ignore -- types on viem dont quite work for setting the tx type but the actual js execution works fine + type: 'cip42', + accessList: [], + chainId: 44378, + data: '0xabcdef', + feeCurrency: '0xcd2a3d9f938e13cd947ec05abc7fe734df8dd826', + gas: BigInt(10), + gatewayFee: BigInt('0x5678'), + gatewayFeeRecipient: '0x1be31a94361a391bbafb2a4ccd704f57dc04d4bb', + maxFeePerGas: BigInt(99), + maxPriorityFeePerGas: BigInt(99), + nonce: 2, + to: '0x588e4b68193001e4d10928660ab4165b813717c0', + value: BigInt(1000000000000000000), + }, + { serializer: celo.serializers?.transaction } + ) + + expect(recoverTransaction(signed)).toMatchInlineSnapshot(` + [ + { + "accessList": [], + "chainId": 44378, + "data": "0xabcdef", + "feeCurrency": "0xcd2a3d9f938e13cd947ec05abc7fe734df8dd826", + "gas": 10, + "gatewayFee": "0x5678", + "gatewayFeeRecipient": "0x1be31a94361a391bbafb2a4ccd704f57dc04d4bb", + "maxFeePerGas": 99, + "maxPriorityFeePerGas": 99, + "nonce": 2, + "r": "0xd8ac17c9add66e5406edcbd925c0fe0257758467c0f1e85c412665f43e472ed6", + "s": "0x7235dcebf2a7082298069dc75a2a8049d9929dbac528e93ad6611047fc7fedc9", + "to": "0x588e4b68193001e4d10928660ab4165b813717c0", + "type": "cip42", + "v": 28, + "value": 1000000000000000000, + "yParity": 1, + }, + "0x1Be31A94361a391bBaFB2a4CCd704F57dc04d4bb", + ] + `) + expect(recoverTransaction(signed)[1]).toEqual(account.address) + }) }) describe('isPriceToLow', () => { diff --git a/packages/sdk/wallets/wallet-base/src/signing-utils.ts b/packages/sdk/wallets/wallet-base/src/signing-utils.ts index ce8aef217..a8c17a8b4 100644 --- a/packages/sdk/wallets/wallet-base/src/signing-utils.ts +++ b/packages/sdk/wallets/wallet-base/src/signing-utils.ts @@ -95,8 +95,16 @@ function signatureFormatter( } { let v = signature.v if (type !== 'celo-legacy' && type !== 'ethereum-legacy') { - v = signature.v === Y_PARITY_EIP_2098 ? 0 : 1 + const vn = BigInt(signature.v) + if (vn === BigInt(Y_PARITY_EIP_2098) || vn === BigInt(0)) { + v = 0 + } else if (vn === BigInt(Y_PARITY_EIP_2098 + 1) || vn === BigInt(1)) { + v = 1 + } else { + throw new Error('Invalid v ' + signature.v) + } } + return { v: trimLeading0x(stringNumberToHex(v)), r: trimLeading0x(makeEven(trimLeadingZero(ensureLeading0x(signature.r.toString('hex'))))), @@ -564,12 +572,15 @@ export function determineTXType(serializedTransaction: string): OldTransactionTy } function vrsForRecovery(vRaw: string, r: string, s: string) { - const v = vRaw === '0x' || hexToNumber(vRaw) === 0 ? Y_PARITY_EIP_2098 : Y_PARITY_EIP_2098 + 1 + const v = + vRaw === '0x' || hexToNumber(vRaw) === 0 || hexToNumber(vRaw) === 27 + ? Y_PARITY_EIP_2098 + : Y_PARITY_EIP_2098 + 1 return { v, r, s, - yParity: v === Y_PARITY_EIP_2098 ? 0 : 1, + yParity: (v - Y_PARITY_EIP_2098) as 0 | 1, } as const } diff --git a/packages/sdk/wallets/wallet-ledger/package.json b/packages/sdk/wallets/wallet-ledger/package.json index 069971c3c..1a437fb9a 100644 --- a/packages/sdk/wallets/wallet-ledger/package.json +++ b/packages/sdk/wallets/wallet-ledger/package.json @@ -32,7 +32,7 @@ "@celo/wallet-remote": "^6.0.2-beta.0", "@ethereumjs/util": "8.0.5", "@ledgerhq/errors": "^6.16.4", - "@ledgerhq/hw-app-eth": "git+https://github.com:celo-org/ledgerjs-hw-app-eth.git", + "@ledgerhq/hw-app-eth": "git+https://github.com:celo-org/ledgerjs-hw-app-eth.git#commit=67c6c3e10929c06e5afd169c16fb8e52f6fda4de", "@ledgerhq/hw-transport": "^6.30.6", "debug": "^4.1.1", "semver": "^7.6.0" @@ -51,5 +51,8 @@ }, "engines": { "node": ">=8.14.2" - } + }, + "bundleDependencies": [ + "@ledgerhq/hw-app-eth" + ] } diff --git a/packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts b/packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts index 40e7f2164..8e3bb37b7 100644 --- a/packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts +++ b/packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts @@ -43,7 +43,7 @@ export class LedgerSigner implements Signer { } async signTransaction( - addToV: number, + _addToV: number, encodedTx: RLPEncodedTx | LegacyEncodedTx ): Promise<{ v: number; r: Buffer; s: Buffer }> { try { @@ -55,15 +55,8 @@ export class LedgerSigner implements Signer { null ) - // EIP155 support. check/recalc signature v value. - const _v = parseInt(signature.v, 16) - // eslint-disable-next-line no-bitwise - if (_v !== addToV && (_v & addToV) !== _v) { - addToV += 1 // add signature v bit. - } - return { - v: _v, + v: parseInt(signature.v, 16), r: ethUtil.toBuffer(ensureLeading0x(signature.r)), s: ethUtil.toBuffer(ensureLeading0x(signature.s)), } @@ -200,15 +193,6 @@ export class LedgerSigner implements Signer { } private provideERC20TokenInformation(tokenInfoData: Buffer) { - // it looks like legacy might need it WITHOUT 0x prefix - const isModern = meetsVersionRequirements(this.appConfiguration.version, { - minimum: LedgerWallet.MIN_VERSION_EIP1559, - }) - - const hexStringTokenInfo = isModern - ? ensureLeading0x(tokenInfoData.toString('hex')) - : trimLeading0x(tokenInfoData.toString('hex')) - - return this.ledger!.provideERC20TokenInformation(hexStringTokenInfo) + return this.ledger!.provideERC20TokenInformation(trimLeading0x(tokenInfoData.toString('hex'))) } } diff --git a/packages/sdk/wallets/wallet-ledger/src/ledger-wallet.test.ts b/packages/sdk/wallets/wallet-ledger/src/ledger-wallet.test.ts index cf3bef77f..885dcca27 100644 --- a/packages/sdk/wallets/wallet-ledger/src/ledger-wallet.test.ts +++ b/packages/sdk/wallets/wallet-ledger/src/ledger-wallet.test.ts @@ -24,13 +24,14 @@ import { VerifyPublicKeyInput, createVerify } from 'crypto' import { readFileSync } from 'fs' import { dirname, join } from 'path' import Web3 from 'web3' -import { rlpEncodedTx } from '../../wallet-base/lib' import { legacyLedgerPublicKeyHex } from './data' import { meetsVersionRequirements } from './ledger-utils' import { AddressValidation, LedgerWallet } from './ledger-wallet' // Update this variable when testing using a physical device const USE_PHYSICAL_LEDGER = process.env.USE_PHYSICAL_LEDGER === 'true' +const hardwareDescribe = USE_PHYSICAL_LEDGER ? describe : describe.skip +const syntheticDescribe = USE_PHYSICAL_LEDGER ? describe.skip : describe // Increase timeout to give developer time to respond on device const TEST_TIMEOUT_IN_MS = USE_PHYSICAL_LEDGER ? 30 * 1000 : 1 * 1000 @@ -144,7 +145,7 @@ const mockLedgerImplementation = (mockForceValidation: () => void, version: stri if (ledgerAddresses[derivationPath]) { const type = determineTXType(ensureLeading0x(data)) // replicate logic from wallet-base/src/wallet-base.ts - const addToV = type === 'celo-legacy' ? chainIdTransformationForSigning(CHAIN_ID) : 27 + const addToV = type === 'celo-legacy' ? chainIdTransformationForSigning(CHAIN_ID) : 0 const hash = getHashFromEncoded(ensureLeading0x(data)) const { r, s, v } = signTransaction( hash, @@ -402,6 +403,14 @@ describe('LedgerWallet class', () => { }) describe('using a known address', () => { + beforeEach(() => { + celoTransaction = { + ...celoTransaction, + from: knownAddress, + to: knownAddress, + } + }) + describe('[eip1559]', () => { beforeEach(async () => { celoTransaction = { @@ -417,13 +426,36 @@ describe('LedgerWallet class', () => { } }) - test( - 'succeeds on cel2 (when ledger version is above minimum)', - async () => { - await expect(wallet.signTransaction(celoTransaction)).resolves.not.toBeUndefined() - }, - TEST_TIMEOUT_IN_MS - ) + describe('succeeds on cel2 (when ledger version is above minimum)', () => { + test( + 'v=0', + async () => { + const signed = await wallet.signTransaction({ + // produces a v=0 + ...celoTransaction, + }) + expect(signed).not.toBeUndefined() + const [_txParams, address] = recoverTransaction(signed.raw) + expect(address.toLowerCase()).toBe(wallet.getAccounts()[0].toLowerCase()) + }, + TEST_TIMEOUT_IN_MS + ) + test( + 'v=1', + async () => { + const signed = await wallet.signTransaction({ + ...celoTransaction, + // produces a v=1 according to device or nah, possibly + // unique to nico's device? + nonce: USE_PHYSICAL_LEDGER ? 2 : 101, + }) + expect(signed).not.toBeUndefined() + const [_txParams, address] = recoverTransaction(signed.raw) + expect(address.toLowerCase()).toBe(wallet.getAccounts()[0].toLowerCase()) + }, + TEST_TIMEOUT_IN_MS + ) + }) // test( 'fails on cel2 (when ledger version is below minimum)', @@ -452,6 +484,19 @@ describe('LedgerWallet class', () => { test( 'on cel1 with old ledger converts to legacy tx', async () => { + if (!hardwareWallet) { + expect(true).toBeTruthy() + return + } + + const { version } = await hardwareWallet.ledger?.getAppConfiguration()! + if ( + meetsVersionRequirements(version, { minimum: LedgerWallet.MIN_VERSION_EIP1559 }) + ) { + expect(true).toBeTruthy() + return + } + wallet = new LedgerWallet( undefined, undefined, @@ -462,8 +507,6 @@ describe('LedgerWallet class', () => { mockForceValidation = jest.fn((): void => { // do nothing }) - mockLedger(wallet, mockForceValidation, LedgerWallet.MIN_VERSION_TOKEN_DATA) - await wallet.init() const warnSpy = jest.spyOn(console, 'warn') // setup complete @@ -575,9 +618,10 @@ describe('LedgerWallet class', () => { feeCurrency: (await kit.contracts.getStableToken(StableToken.cUSD)).address, } }) - describe('with old ledger app', () => { - describe('on Cel2 with old app version', () => { - beforeEach(async () => { + describe('on Cel2 with old app version', () => { + test( + 'fails', + async () => { wallet = new LedgerWallet( undefined, undefined, @@ -590,40 +634,21 @@ describe('LedgerWallet class', () => { }) mockLedger(wallet, mockForceValidation, LedgerWallet.MIN_VERSION_TOKEN_DATA) await wallet.init() - }) - test( - 'fails', - async () => { - await expect( - wallet.signTransaction(celoTransaction) - ).rejects.toThrowErrorMatchingInlineSnapshot( - `"celo ledger app version must be at least 1.2.0 to sign transactions supported on celo after the L2 upgrade"` - ) - }, - TEST_TIMEOUT_IN_MS - ) - }) - describe('on celo l1 with old app version', () => { - test( - 'physical device only', - async () => { - if (!hardwareWallet) { - expect(true).toBeTruthy() - return - } - const signedTx = await wallet.signTransaction(celoTransaction) - const [_tx, recoveredSigner] = recoverTransaction(signedTx.raw) - expect(normalizeAddressWith0x(recoveredSigner)).toEqual( - normalizeAddressWith0x(knownAddress) - ) - }, - TEST_TIMEOUT_IN_MS - ) - - test( - 'succeeds with warning', - async () => { + await expect( + wallet.signTransaction(celoTransaction) + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"celo ledger app version must be at least 1.2.0 to sign transactions supported on celo after the L2 upgrade"` + ) + }, + TEST_TIMEOUT_IN_MS + ) + }) + describe('on celo l1 with old app version', () => { + test( + 'succeeds', + async () => { + if (!USE_PHYSICAL_LEDGER) { wallet = new LedgerWallet( undefined, undefined, @@ -636,28 +661,148 @@ describe('LedgerWallet class', () => { }) mockLedger(wallet, mockForceValidation, LedgerWallet.MIN_VERSION_TOKEN_DATA) await wallet.init() + } + + const signedTx = await wallet.signTransaction(celoTransaction) + const [_tx, recoveredSigner] = recoverTransaction(signedTx.raw) + expect(normalizeAddressWith0x(recoveredSigner)).toEqual( + normalizeAddressWith0x(knownAddress) + ) + }, + TEST_TIMEOUT_IN_MS + ) + }) + + describe('succeeds with warning', () => { + syntheticDescribe('synthetic', () => { + beforeEach(async () => { + wallet = new LedgerWallet( + undefined, + undefined, + undefined, + AddressValidation.never, + false + ) + mockForceValidation = jest.fn((): void => { + // do nothing + }) + mockLedger(wallet, mockForceValidation, LedgerWallet.MIN_VERSION_TOKEN_DATA) + await wallet.init() + }) + + test('v=0', async () => { + const warnSpy = jest.spyOn(console, 'warn') + // setup complete + + const tx = await wallet.signTransaction(celoTransaction) + expect(tx).toMatchInlineSnapshot(` + { + "raw": "0xf87f80636394874069fa1eb16d44d622f2e0ca25eea172369bc1808094588e4b68193001e4d10928660ab4165b813717c0880de0b6b3a76400008083015e09a024c4b1d027c50d2e847d371cd902d3e22c9fa10fcbd59e9c5a854282afed34daa0686180d75830ea223c3ed1ca12613d029bc3613b4b5b51a724b33067491c2398", + "tx": { + "feeCurrency": "0x874069fa1eb16d44d622f2e0ca25eea172369bc1", + "gas": "0x63", + "hash": "0x302b12585b4a17317e9affcbe0abd0cd8cd39ef402108625085485b1c1d92d71", + "input": "0x", + "nonce": "0", + "r": "0x24c4b1d027c50d2e847d371cd902d3e22c9fa10fcbd59e9c5a854282afed34da", + "s": "0x686180d75830ea223c3ed1ca12613d029bc3613b4b5b51a724b33067491c2398", + "to": "0x588e4b68193001e4d10928660ab4165b813717c0", + "v": "0x015e09", + "value": "0x0de0b6b3a7640000", + }, + "type": "celo-legacy", + } + `) + expect(recoverTransaction(tx.raw)[1].toLowerCase()).toBe( + wallet.getAccounts()[0].toLowerCase() + ) + expect(warnSpy).toHaveBeenCalledWith( + 'Upgrade your celo ledger app to at least 1.2.0 before cel2 transition' + ) + }) + test('v=1', async () => { + const warnSpy = jest.spyOn(console, 'warn') + // setup complete + + const tx = await wallet.signTransaction({ + ...celoTransaction, + nonce: 1, + }) + expect(tx).toMatchInlineSnapshot(` + { + "raw": "0xf87f01636394874069fa1eb16d44d622f2e0ca25eea172369bc1808094588e4b68193001e4d10928660ab4165b813717c0880de0b6b3a76400008083015e0aa07f4b5fda6eb400f44a22efae0f394b5237c0edfeab75f5bee5952ccbdf1c75a5a03d1f8e0cbfd80027a8523caf3600bd4cb3188761b1c11924607b060dc46a726f", + "tx": { + "feeCurrency": "0x874069fa1eb16d44d622f2e0ca25eea172369bc1", + "gas": "0x63", + "hash": "0x1a60bc35a24e3457bc15fdcbb37f77424f95697cda005d722d2c6e4d3bb95da0", + "input": "0x", + "nonce": "1", + "r": "0x7f4b5fda6eb400f44a22efae0f394b5237c0edfeab75f5bee5952ccbdf1c75a5", + "s": "0x3d1f8e0cbfd80027a8523caf3600bd4cb3188761b1c11924607b060dc46a726f", + "to": "0x588e4b68193001e4d10928660ab4165b813717c0", + "v": "0x015e0a", + "value": "0x0de0b6b3a7640000", + }, + "type": "celo-legacy", + } + `) + expect(recoverTransaction(tx.raw)[1].toLowerCase()).toBe( + wallet.getAccounts()[0].toLowerCase() + ) + expect(warnSpy).toHaveBeenCalledWith( + 'Upgrade your celo ledger app to at least 1.2.0 before cel2 transition' + ) + }) + }) + + hardwareDescribe('physical device', () => { + test( + 'v=0', + async () => { + const warnSpy = jest.spyOn(console, 'warn') + const tx = await wallet.signTransaction(celoTransaction) + // @ts-expect-error + expect(tx.type).toBe('celo-legacy') + expect(tx.tx.nonce).toBe('0') + expect(BigInt(tx.tx.value)).toBe(BigInt(celoTransaction.value as string)) + expect(parseInt(tx.tx.v, 16)).toBeGreaterThanOrEqual( + chainIdTransformationForSigning(CHAIN_ID) + ) + // @ts-expect-error + expect(tx.tx.feeCurrency.toLowerCase()).toBe( + celoTransaction.feeCurrency?.toLowerCase() + ) + expect(recoverTransaction(tx.raw)[1].toLowerCase()).toBe( + wallet.getAccounts()[0].toLowerCase() + ) + expect(warnSpy).toHaveBeenCalledWith( + 'Upgrade your celo ledger app to at least 1.2.0 before cel2 transition' + ) + }, + TEST_TIMEOUT_IN_MS + ) + test( + 'v=1', + async () => { const warnSpy = jest.spyOn(console, 'warn') - // setup complete - - await expect(wallet.signTransaction(celoTransaction)).resolves - .toMatchInlineSnapshot(` - { - "raw": "0xf87f80636394874069fa1eb16d44d622f2e0ca25eea172369bc1808094588e4b68193001e4d10928660ab4165b813717c0880de0b6b3a76400008083015e09a024c4b1d027c50d2e847d371cd902d3e22c9fa10fcbd59e9c5a854282afed34daa0686180d75830ea223c3ed1ca12613d029bc3613b4b5b51a724b33067491c2398", - "tx": { - "feeCurrency": "0x874069fa1eb16d44d622f2e0ca25eea172369bc1", - "gas": "0x63", - "hash": "0x302b12585b4a17317e9affcbe0abd0cd8cd39ef402108625085485b1c1d92d71", - "input": "0x", - "nonce": "0", - "r": "0x24c4b1d027c50d2e847d371cd902d3e22c9fa10fcbd59e9c5a854282afed34da", - "s": "0x686180d75830ea223c3ed1ca12613d029bc3613b4b5b51a724b33067491c2398", - "to": "0x588e4b68193001e4d10928660ab4165b813717c0", - "v": "0x015e09", - "value": "0x0de0b6b3a7640000", - }, - "type": "celo-legacy", - } - `) + const tx = await wallet.signTransaction({ + ...celoTransaction, + nonce: 1, + }) + // @ts-expect-error + expect(tx.type).toBe('celo-legacy') + expect(tx.tx.nonce).toBe('1') + expect(BigInt(tx.tx.value)).toBe(BigInt(celoTransaction.value as string)) + expect(parseInt(tx.tx.v, 16)).toBeGreaterThanOrEqual( + chainIdTransformationForSigning(CHAIN_ID) + ) + // @ts-expect-error + expect(tx.tx.feeCurrency.toLowerCase()).toBe( + celoTransaction.feeCurrency?.toLowerCase() + ) + expect(recoverTransaction(tx.raw)[1].toLowerCase()).toBe( + wallet.getAccounts()[0].toLowerCase() + ) expect(warnSpy).toHaveBeenCalledWith( 'Upgrade your celo ledger app to at least 1.2.0 before cel2 transition' ) @@ -666,45 +811,46 @@ describe('LedgerWallet class', () => { ) }) }) - describe('with new ledger app', () => { - test( - 'fails with helpful error', - async () => { - await expect( - wallet.signTransaction(celoTransaction) - ).rejects.toThrowErrorMatchingInlineSnapshot( - `"celo ledger app above 1.2.0 cannot serialize legacy celo transactions. Replace "gasPrice" with "maxFeePerGas"."` - ) - }, - TEST_TIMEOUT_IN_MS - ) - }) }) - - describe('[cip64]', () => { - const kit = newKit('https://alfajores-forno.celo-testnet.org') - beforeEach(async () => { - celoTransaction = { - from: knownAddress, - to: otherAddress, - chainId: CHAIN_ID, - value: Web3.utils.toWei('1', 'ether'), - nonce: 0, - gas: 99, - maxFeePerGas: 99, - maxPriorityFeePerGas: 99, - feeCurrency: (await kit.contracts.getStableToken(StableToken.cUSD)).address, - } - }) - + hardwareDescribe('with new ledger app', () => { test( - 'succeeds', + 'fails with helpful error', async () => { - jest - .spyOn(wallet.ledger!, 'provideERC20TokenInformation') - .mockImplementationOnce(async () => true) - await expect(wallet.signTransaction(celoTransaction)).resolves - .toMatchInlineSnapshot(` + await expect( + wallet.signTransaction(celoTransaction) + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"celo ledger app above 1.2.0 cannot serialize legacy celo transactions. Replace "gasPrice" with "maxFeePerGas"."` + ) + }, + TEST_TIMEOUT_IN_MS + ) + }) + }) + + syntheticDescribe('[cip64] synthetic', () => { + const kit = newKit('https://alfajores-forno.celo-testnet.org') + beforeEach(async () => { + celoTransaction = { + from: knownAddress, + to: otherAddress, + chainId: CHAIN_ID, + value: Web3.utils.toWei('1', 'ether'), + nonce: 0, + gas: 99, + maxFeePerGas: 99, + maxPriorityFeePerGas: 99, + feeCurrency: (await kit.contracts.getStableToken(StableToken.cUSD)).address, + } + }) + + test( + 'succeeds', + async () => { + jest + .spyOn(wallet.ledger!, 'provideERC20TokenInformation') + .mockImplementation(() => Promise.resolve(true)) + + await expect(wallet.signTransaction(celoTransaction)).resolves.toMatchInlineSnapshot(` { "raw": "0x7bf87f82aef38063636394588e4b68193001e4d10928660ab4165b813717c0880de0b6b3a764000080c094874069fa1eb16d44d622f2e0ca25eea172369bc101a0254f952c5223c30039f7f845778d7aac558464ce2971fd09883df34913eb6dfca037a78571ae1a44d86bac7269e3a845990a49ad5fb60a5ec1fcaba428693558c0", "tx": { @@ -726,66 +872,109 @@ describe('LedgerWallet class', () => { } `) - expect(wallet.ledger!.provideERC20TokenInformation).toHaveBeenCalledWith( - `0x06612063555344874069fa1eb16d44d622f2e0ca25eea172369bc1000000120000aef33045022100a885480c357fd6ec64ed532656a7e988198fdf4e2cf4632408f2d65561189872022009fd78725055fc68af16e151516ba29625e3e1c74ceab3da1bcabd6015e3f6e8` - ) - }, - TEST_TIMEOUT_IN_MS - ) - }) - describe.skip('[cip66]', () => { - const kit = newKit('https://alfajores-forno.celo-testnet.org') - beforeEach(async () => { - celoTransaction = { - from: knownAddress, - to: otherAddress, - chainId: CHAIN_ID, - value: Web3.utils.toWei('1', 'ether'), - nonce: 0, - gas: 99, - maxFeePerGas: 99, - maxPriorityFeePerGas: 99, - feeCurrency: (await kit.contracts.getStableToken(StableToken.cUSD)).address, - } - }) + expect(wallet.ledger!.provideERC20TokenInformation).toHaveBeenCalledWith( + `06612063555344874069fa1eb16d44d622f2e0ca25eea172369bc1000000120000aef33045022100a885480c357fd6ec64ed532656a7e988198fdf4e2cf4632408f2d65561189872022009fd78725055fc68af16e151516ba29625e3e1c74ceab3da1bcabd6015e3f6e8` + ) + }, + TEST_TIMEOUT_IN_MS + ) + }) - test( - 'gives warning', - async () => { - await expect(wallet.signTransaction(celoTransaction)).resolves.not.toBeUndefined() - }, - TEST_TIMEOUT_IN_MS - ) + hardwareDescribe('[cip64] device', () => { + const kit = newKit('https://alfajores-forno.celo-testnet.org') + beforeEach(async () => { + celoTransaction = { + from: knownAddress, + to: otherAddress, + chainId: CHAIN_ID, + value: Web3.utils.toWei('1', 'ether'), + nonce: 0, + gas: 99, + maxFeePerGas: 99, + maxPriorityFeePerGas: 99, + feeCurrency: (await kit.contracts.getStableToken(StableToken.cUSD)).address, + } }) - }) - describe('when calling signPersonalMessage', () => { test( 'succeeds', async () => { - const hexStr: string = ACCOUNT_ADDRESS1 - const signedMessage = await wallet.signPersonalMessage(knownAddress, hexStr) - expect(signedMessage).not.toBeUndefined() - const valid = verifySignature(hexStr, signedMessage, knownAddress) - expect(valid).toBeTruthy() + jest.spyOn(wallet.ledger!, 'provideERC20TokenInformation') + const tx = await wallet.signTransaction(celoTransaction) + // @ts-expect-error + expect(tx.type).toBe('cip64') + expect(tx.tx.nonce).toBe('0') + expect(BigInt(tx.tx.value)).toBe(BigInt(celoTransaction.value as string)) + const v = parseInt(tx.tx.v, 16) + expect(v === 0 || v === 1).toBe(true) + // @ts-expect-error + expect(tx.tx.feeCurrency.toLowerCase()).toBe( + celoTransaction.feeCurrency?.toLowerCase() + ) + expect(recoverTransaction(tx.raw)[1].toLowerCase()).toBe( + wallet.getAccounts()[0].toLowerCase() + ) + + expect(wallet.ledger!.provideERC20TokenInformation).toHaveBeenCalledWith( + `06612063555344874069fa1eb16d44d622f2e0ca25eea172369bc1000000120000aef33045022100a885480c357fd6ec64ed532656a7e988198fdf4e2cf4632408f2d65561189872022009fd78725055fc68af16e151516ba29625e3e1c74ceab3da1bcabd6015e3f6e8` + ) }, TEST_TIMEOUT_IN_MS ) }) - describe('when calling signTypedData', () => { - test.skip( - 'succeeds', + describe.skip('[cip66]', () => { + const kit = newKit('https://alfajores-forno.celo-testnet.org') + beforeEach(async () => { + celoTransaction = { + from: knownAddress, + to: otherAddress, + chainId: CHAIN_ID, + value: Web3.utils.toWei('1', 'ether'), + nonce: 0, + gas: 99, + maxFeePerGas: 99, + maxPriorityFeePerGas: 99, + feeCurrency: (await kit.contracts.getStableToken(StableToken.cUSD)).address, + } + }) + + test( + 'gives warning', async () => { - const signedMessage = await wallet.signTypedData(knownAddress, TYPED_DATA) - expect(signedMessage).not.toBeUndefined() - const valid = verifyEIP712TypedDataSigner(TYPED_DATA, signedMessage, knownAddress) - expect(valid).toBeTruthy() + await expect(wallet.signTransaction(celoTransaction)).resolves.not.toBeUndefined() }, TEST_TIMEOUT_IN_MS ) }) }) + + describe('when calling signPersonalMessage', () => { + test( + 'succeeds', + async () => { + const hexStr: string = ACCOUNT_ADDRESS1 + const signedMessage = await wallet.signPersonalMessage(knownAddress, hexStr) + expect(signedMessage).not.toBeUndefined() + const valid = verifySignature(hexStr, signedMessage, knownAddress) + expect(valid).toBeTruthy() + }, + TEST_TIMEOUT_IN_MS + ) + }) + + describe('when calling signTypedData', () => { + test( + 'succeeds', + async () => { + const signedMessage = await wallet.signTypedData(knownAddress, TYPED_DATA) + expect(signedMessage).not.toBeUndefined() + const valid = verifyEIP712TypedDataSigner(TYPED_DATA, signedMessage, knownAddress) + expect(valid).toBeTruthy() + }, + TEST_TIMEOUT_IN_MS + ) + }) }) }) @@ -916,44 +1105,3 @@ describe('LedgerWallet class', () => { }) }) }) - -describe('patch-package @ledgerhq/hw-app-eth', () => { - test('was applied correctly', async () => { - const { decodeTxInfo } = await import('@ledgerhq/hw-app-eth/lib/utils') - - const kit = newKit('https://alfajores-forno.celo-testnet.org') - const celoTransaction = { - from: ACCOUNT_ADDRESS1, - to: ACCOUNT_ADDRESS2, - chainId: CHAIN_ID, - value: Web3.utils.toWei('1', 'ether'), - nonce: 0, - gas: 99, - maxFeePerGas: 99, - maxPriorityFeePerGas: 99, - feeCurrency: (await kit.contracts.getStableToken(StableToken.cUSD)).address, - } - const serialized = rlpEncodedTx(celoTransaction) - const rawTx = Buffer.from(trimLeading0x(serialized.rlpEncode), 'hex') - let ledgerDecoded: ReturnType - expect(() => { - ledgerDecoded = decodeTxInfo(rawTx) - }).not.toThrow(/invalid rlp data/) - expect(ledgerDecoded!.txType).toEqual(0x7b) - expect(ledgerDecoded!.chainId.toNumber()).toEqual(CHAIN_ID) - expect(ledgerDecoded!.decodedTx).toMatchInlineSnapshot(` - { - "chainId": { - "data": [ - 174, - 243, - ], - "type": "Buffer", - }, - "data": "0x", - "feeCurrency": "0x874069fa1eb16d44d622f2e0ca25eea172369bc1", - "to": "0x588e4b68193001e4d10928660ab4165b813717c0", - } - `) - }) -}) diff --git a/packages/sdk/wallets/wallet-ledger/src/ledger-wallet.ts b/packages/sdk/wallets/wallet-ledger/src/ledger-wallet.ts index 1fa49baf9..58bbc8f98 100644 --- a/packages/sdk/wallets/wallet-ledger/src/ledger-wallet.ts +++ b/packages/sdk/wallets/wallet-ledger/src/ledger-wallet.ts @@ -127,23 +127,25 @@ export class LedgerWallet extends RemoteWallet implements ReadOnly ) } // but if not celo as layer 2 and as layer 1 are different - } else if (this.isCel2) { - throw new Error( - `celo ledger app version must be at least ${LedgerWallet.MIN_VERSION_EIP1559} to sign transactions supported on celo after the L2 upgrade` - ) } else { - // the l1 legacy case - console.warn( - `Upgrade your celo ledger app to at least ${LedgerWallet.MIN_VERSION_EIP1559} before cel2 transition` - ) - if (!txParams.gasPrice) { - // this version of app only supports legacy so must have gasPrice - txParams.gasPrice = txParams.maxFeePerGas - delete txParams.maxFeePerGas - delete txParams.maxPriorityFeePerGas - console.info('automatically converting to legacy transaction') + if (this.isCel2) { + throw new Error( + `celo ledger app version must be at least ${LedgerWallet.MIN_VERSION_EIP1559} to sign transactions supported on celo after the L2 upgrade` + ) + } else { + // the l1 legacy case + console.warn( + `Upgrade your celo ledger app to at least ${LedgerWallet.MIN_VERSION_EIP1559} before cel2 transition` + ) + if (!txParams.gasPrice) { + // this version of app only supports legacy so must have gasPrice + txParams.gasPrice = txParams.maxFeePerGas + delete txParams.maxFeePerGas + delete txParams.maxPriorityFeePerGas + console.info('automatically converting to legacy transaction') + } + return encode_deprecated_celo_legacy_type_only_for_temporary_ledger_compat(txParams) } - return encode_deprecated_celo_legacy_type_only_for_temporary_ledger_compat(txParams) } } diff --git a/packages/sdk/wallets/wallet-local/src/local-wallet.test.ts b/packages/sdk/wallets/wallet-local/src/local-wallet.test.ts index 7386f0a97..e0f0aad20 100644 --- a/packages/sdk/wallets/wallet-local/src/local-wallet.test.ts +++ b/packages/sdk/wallets/wallet-local/src/local-wallet.test.ts @@ -300,6 +300,11 @@ describe('Local wallet class', () => { "type": "cip66", } `) + expect( + recoverTransaction( + '0x7af88382ad5a8063630a94588e4b68193001e4d10928660ab4165b813717c0880de0b6b3a764000083abcdefc094cd2a3d9f938e13cd947ec05abc7fe734df8dd8265c01a00fb404c1a62ab54b47b4ca07f5ac7e7b233be6cd173294c0b1f3a209c36f6265a05ac38f9ddd67ecf936f2dfea2be5f641959e2a66545fffb01ebd8c925ac23b89' + )[1].toLowerCase() + ).toBe(wallet.getAccounts()[0].toLowerCase()) }) test('succeeds with cip64', async () => { const recoverTransactionCIP64 = { diff --git a/yarn.lock b/yarn.lock index 322d24a68..4e1b88a59 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2308,7 +2308,7 @@ __metadata: "@celo/wallet-remote": "npm:^6.0.2-beta.0" "@ethereumjs/util": "npm:8.0.5" "@ledgerhq/errors": "npm:^6.16.4" - "@ledgerhq/hw-app-eth": "git+https://github.com:celo-org/ledgerjs-hw-app-eth.git" + "@ledgerhq/hw-app-eth": "git+https://github.com:celo-org/ledgerjs-hw-app-eth.git#commit=67c6c3e10929c06e5afd169c16fb8e52f6fda4de" "@ledgerhq/hw-transport": "npm:^6.30.6" "@ledgerhq/hw-transport-node-hid": "npm:^6.28.5" "@noble/curves": "npm:^1.4.0" @@ -3944,6 +3944,16 @@ __metadata: languageName: node linkType: hard +"@ledgerhq/cryptoassets-evm-signatures@npm:^13.5.1": + version: 13.5.1 + resolution: "@ledgerhq/cryptoassets-evm-signatures@npm:13.5.1" + dependencies: + "@ledgerhq/live-env": "npm:^2.4.0" + axios: "npm:1.7.7" + checksum: 8e9889a0a4c53afcbac0d42eeff5617ac37d9a69528861080a6dbada0a4fced10b5e7157c27224fb854776f5f248d2a77baa33c39525064b440285434422b83b + languageName: node + linkType: hard + "@ledgerhq/cryptoassets@npm:^13.1.1": version: 13.1.1 resolution: "@ledgerhq/cryptoassets@npm:13.1.1" @@ -4018,6 +4028,21 @@ __metadata: languageName: node linkType: hard +"@ledgerhq/domain-service@npm:^1.2.10": + version: 1.2.10 + resolution: "@ledgerhq/domain-service@npm:1.2.10" + dependencies: + "@ledgerhq/errors": "npm:^6.19.1" + "@ledgerhq/logs": "npm:^6.12.0" + "@ledgerhq/types-live": "npm:^6.52.4" + axios: "npm:1.7.7" + eip55: "npm:^2.1.1" + react: "npm:^18.2.0" + react-dom: "npm:^18.2.0" + checksum: a46d546bd68ee3f7247e63e89b3425ab7df9d62f75501b6db7a13b1a319fb40cb2272c638ddc6c1bee973f66bd70fa4156d3287fa31e20a5bb2d0164d59aaf61 + languageName: node + linkType: hard + "@ledgerhq/errors@npm:^6.16.3": version: 6.16.3 resolution: "@ledgerhq/errors@npm:6.16.3" @@ -4059,6 +4084,19 @@ __metadata: languageName: node linkType: hard +"@ledgerhq/evm-tools@npm:^1.2.4": + version: 1.2.4 + resolution: "@ledgerhq/evm-tools@npm:1.2.4" + dependencies: + "@ledgerhq/cryptoassets-evm-signatures": "npm:^13.5.1" + "@ledgerhq/live-env": "npm:^2.4.0" + axios: "npm:1.7.7" + crypto-js: "npm:4.2.0" + ethers: "npm:5.7.2" + checksum: 5e1e213f39b337a91858ba94418ed816d7fd7591c2798da1754cca6ac96f395c80fbb5e8c41ed568383b669c7eed59e998e5a5f81bee1c92f17f1a4895f97772 + languageName: node + linkType: hard + "@ledgerhq/hw-app-eth@git+https://github.com:celo-org/ledgerjs-hw-app-eth.git": version: 6.37.1 resolution: "@ledgerhq/hw-app-eth@https://github.com:celo-org/ledgerjs-hw-app-eth.git#commit=49bdd4f163f5ff73daa9f54f8e46aa2882b85f44" @@ -4081,6 +4119,27 @@ __metadata: languageName: node linkType: hard +"@ledgerhq/hw-app-eth@git+https://github.com:celo-org/ledgerjs-hw-app-eth.git#commit=67c6c3e10929c06e5afd169c16fb8e52f6fda4de": + version: 6.40.3 + resolution: "@ledgerhq/hw-app-eth@https://github.com:celo-org/ledgerjs-hw-app-eth.git#commit=67c6c3e10929c06e5afd169c16fb8e52f6fda4de" + dependencies: + "@ethersproject/abi": "npm:^5.5.0" + "@ethersproject/rlp": "npm:^5.5.0" + "@ledgerhq/cryptoassets-evm-signatures": "npm:^13.5.1" + "@ledgerhq/domain-service": "npm:^1.2.10" + "@ledgerhq/errors": "npm:^6.19.1" + "@ledgerhq/evm-tools": "npm:^1.2.4" + "@ledgerhq/hw-transport": "npm:^6.31.4" + "@ledgerhq/hw-transport-mocker": "npm:^6.29.4" + "@ledgerhq/logs": "npm:^6.12.0" + "@ledgerhq/types-live": "npm:^6.52.4" + axios: "npm:1.7.7" + bignumber.js: "npm:^9.1.2" + semver: "npm:^7.3.5" + checksum: f4385680d1fd003bb95eb8bf4baeabccd01a08d9ced3d28a2bf78b6ac7000b45f07cc829beab9e5bbaf423fd2d41befff3e3e6110ac5606e81cd5a1b5ae14f15 + languageName: node + linkType: hard + "@ledgerhq/hw-transport-mocker@npm:^6.29.0": version: 6.29.0 resolution: "@ledgerhq/hw-transport-mocker@npm:6.29.0" @@ -4092,6 +4151,17 @@ __metadata: languageName: node linkType: hard +"@ledgerhq/hw-transport-mocker@npm:^6.29.4": + version: 6.29.4 + resolution: "@ledgerhq/hw-transport-mocker@npm:6.29.4" + dependencies: + "@ledgerhq/hw-transport": "npm:^6.31.4" + "@ledgerhq/logs": "npm:^6.12.0" + rxjs: "npm:^7.8.1" + checksum: 6f1568b1723ee6964872b09b712714bacf33c87e83413a33420b7ba11e3c30fa6786f02d2cf7b8bc9b3560f4b5c3b166017d5e0a960267a7824a153687fe32ed + languageName: node + linkType: hard + "@ledgerhq/hw-transport-node-hid-noevents@npm:^6.29.5": version: 6.29.5 resolution: "@ledgerhq/hw-transport-node-hid-noevents@npm:6.29.5" @@ -4208,6 +4278,16 @@ __metadata: languageName: node linkType: hard +"@ledgerhq/live-env@npm:^2.4.0": + version: 2.4.0 + resolution: "@ledgerhq/live-env@npm:2.4.0" + dependencies: + rxjs: "npm:^7.8.1" + utility-types: "npm:^3.10.0" + checksum: 825337025181bb97ac9c55f413a0cf0b2fff2be62f53b5230d328f592fd0b8b9ee4e2d979bf55f576361b880dd5f1424a9331b2da597414c111c213ab7a15dba + languageName: node + linkType: hard + "@ledgerhq/logs@npm:^6.12.0": version: 6.12.0 resolution: "@ledgerhq/logs@npm:6.12.0" @@ -4239,6 +4319,16 @@ __metadata: languageName: node linkType: hard +"@ledgerhq/types-live@npm:^6.52.4": + version: 6.52.4 + resolution: "@ledgerhq/types-live@npm:6.52.4" + dependencies: + bignumber.js: "npm:^9.1.2" + rxjs: "npm:^7.8.1" + checksum: 54288b5b334f0e9e57e5dbea9e8f9a86391e2e2daea2db755ee812dd9e687d45e426aab8737bac1c1fe308281ec170f75aca8845f2a3dba0201dc32a3dcdec1a + languageName: node + linkType: hard + "@manypkg/find-root@npm:^1.1.0": version: 1.1.0 resolution: "@manypkg/find-root@npm:1.1.0" @@ -7555,6 +7645,17 @@ __metadata: languageName: node linkType: hard +"axios@npm:1.7.7": + version: 1.7.7 + resolution: "axios@npm:1.7.7" + dependencies: + follow-redirects: "npm:^1.15.6" + form-data: "npm:^4.0.0" + proxy-from-env: "npm:^1.1.0" + checksum: 7f875ea13b9298cd7b40fd09985209f7a38d38321f1118c701520939de2f113c4ba137832fe8e3f811f99a38e12c8225481011023209a77b0c0641270e20cde1 + languageName: node + linkType: hard + "axios@npm:^1.3.4, axios@npm:^1.6.0, axios@npm:^1.6.5": version: 1.6.8 resolution: "axios@npm:1.6.8"