Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: ledger-v backport #443

Merged
merged 11 commits into from
Nov 19, 2024
6 changes: 6 additions & 0 deletions .changeset/twenty-humans-beg.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@celo/wallet-ledger': patch
'@celo/viem-account-ledger': patch
---

Safer handling of v from device
1 change: 1 addition & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package.json

packages/**/dist
packages/**/lib
packages/**/lib-es

# Needed because we have packages/celotool/src/lib
!packages/celotool/src/**
Expand Down
6 changes: 2 additions & 4 deletions codecov.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@ coverage:
status:
patch:
default:
# basic
target: auto
threshold: 0%
base: auto
target: 80%
only_pulls: true

comment:
layout: 'header, diff, flags, components'

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ Signer.computeSharedSecret

#### Defined in

[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)
[wallet-ledger/src/ledger-signer.ts:207](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts#L207)

___

Expand All @@ -96,7 +96,7 @@ Signer.decrypt

#### Defined in

[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)
[wallet-ledger/src/ledger-signer.ts:201](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts#L201)

___

Expand Down Expand Up @@ -138,7 +138,7 @@ Signer.signPersonalMessage

#### Defined in

[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)
[wallet-ledger/src/ledger-signer.ts:96](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts#L96)

___

Expand Down Expand Up @@ -187,4 +187,4 @@ Signer.signTypedData

#### Defined in

[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)
[wallet-ledger/src/ledger-signer.ts:116](https://github.com/celo-org/developer-tooling/blob/master/packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts#L116)
25 changes: 21 additions & 4 deletions packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,33 @@
try {
const validatedDerivationPath = await this.getValidatedDerivationPath()
await this.checkForKnownToken(encodedTx)
const signature = await this.ledger!.signTransaction(
let {
r,
s,
v: _v,
} = await this.ledger!.signTransaction(
validatedDerivationPath,
trimLeading0x(encodedTx.rlpEncode), // the ledger requires the rlpEncode without the leading 0x
null
)

if (typeof _v === 'string' && (_v === '' || _v === '0x')) {
console.warn(

Check warning on line 63 in packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts

View check run for this annotation

Codecov / codecov/patch

packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts#L63

Added line #L63 was not covered by tests
`ledger-signer@signTransaction: signature \`v\` was malformed \`${_v}\`. Replaced with "0x0"`
)
_v = '0x0'

Check warning on line 66 in packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts

View check run for this annotation

Codecov / codecov/patch

packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts#L66

Added line #L66 was not covered by tests
}
const v = typeof _v === 'string' ? parseInt(ensureLeading0x(_v), 16) : _v
if (isNaN(v)) {
throw new Error(

Check warning on line 70 in packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts

View check run for this annotation

Codecov / codecov/patch

packages/sdk/wallets/wallet-ledger/src/ledger-signer.ts#L70

Added line #L70 was not covered by tests
`ledger-signer@signTransaction: signature \`v\` was malformed and was parsed to NaN \`${_v}\``
)
}

return {
v: parseInt(signature.v, 16),
r: ethUtil.toBuffer(ensureLeading0x(signature.r)),
s: ethUtil.toBuffer(ensureLeading0x(signature.s)),
v,
r: ethUtil.toBuffer(ensureLeading0x(r)),
s: ethUtil.toBuffer(ensureLeading0x(s)),
}
} catch (error: unknown) {
if (error instanceof TransportStatusError) {
Expand Down
3 changes: 2 additions & 1 deletion packages/viem-account-ledger/.gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
lib/
lib-es/
tmp/
.tmp/
.env
.env
20 changes: 11 additions & 9 deletions packages/viem-account-ledger/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@
"version": "1.0.0-beta.1",
"description": "Helper library to make ledger<->viem interactions easier",
"type": "module",
"exports": {
".": "./lib/index.js"
},
"types": "./lib/index.d.ts",
"main": "lib/index.js",
"module": "lib-es/index.js",
"types": "lib/index.d.ts",
"author": "cLabs",
"license": "Apache-2.0",
"homepage": "https://docs.celo.org/developer/tools",
Expand All @@ -18,12 +17,14 @@
"ledger"
],
"scripts": {
"build": "yarn run --top-level tsc -b .",
"clean": "yarn run --top-level tsc -b . --clean",
"docs": "yarn run --top-level typedoc",
"build": "yarn build:cjs && yarn build:esm",
"build:cjs": "yarn --top-level run tsc -m commonjs",
"build:esm": "yarn --top-level run tsc -m ES6 --outDir lib-es",
"clean": "yarn --top-level run tsc -b . --clean && yarn run rimraf lib lib-es",
"docs": "yarn --top-level run typedoc",
"test": "yarn run vitest",
"lint": "yarn run --top-level eslint -c .eslintrc.cjs ",
"prepublishOnly": "yarn build"
"lint": "yarn --top-level run eslint -c .eslintrc.cjs ",
"prepublishOnly": "yarn clean && yarn build"
},
"peerDependencies": {
"@ledgerhq/hw-transport-node-hid": "^6.x",
Expand All @@ -45,6 +46,7 @@
"@ledgerhq/hw-transport-node-hid": "^6.29.5",
"@vitest/coverage-v8": "2.1.2",
"dotenv": "^8.2.0",
"rimraf": "^4.4.1",
"viem": "^2.21.14",
"vitest": "^2.1.2"
},
Expand Down
Loading
Loading