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

refactor: refactor sign method to allow unidentified lock scripts #2983

Draft
wants to merge 26 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
e6c1c43
chore(deps): update dependency lerna to v7.4.2 (#314)
renovate[bot] Nov 27, 2023
52acea6
chore(deps): update dependency jest-when to v3.6.0 (#313)
renovate[bot] Nov 27, 2023
b894c1a
chore(deps): update dependency jest-styled-components to v7.2.0 (#312)
renovate[bot] Nov 28, 2023
89d58c3
fix(deps): update dependency @ckb-lumos/bi to v0.21.0-next.3 (#317)
renovate[bot] Nov 28, 2023
28ec92f
chore(deps): update dependency eslint-plugin-import to v2.29.0 (#309)
renovate[bot] Nov 28, 2023
e2f63d1
fix(deps): update dependency @ckb-lumos/codec to v0.21.0-next.3 (#319)
renovate[bot] Nov 28, 2023
799eb2f
fix(deps): update dependency @ckb-lumos/hd to v0.21.0-next.3 (#322)
renovate[bot] Nov 28, 2023
2cd52f2
fix(deps): update dependency @ckb-lumos/common-scripts to v0.21.0-nex…
renovate[bot] Nov 28, 2023
9d3d1b5
fix(deps): update dependency @ckb-lumos/ckb-indexer to v0.21.0-next.3…
renovate[bot] Nov 28, 2023
1dede13
fix(deps): update dependency @ckb-lumos/config-manager to v0.21.0-nex…
renovate[bot] Nov 28, 2023
633f4ed
Merge branch 'develop' of https://github.com/magickbase/neuron into d…
Keith-CY Dec 8, 2023
3dfc45a
chore(deps): update dependency react-i18next to v13.5.0 (#325)
renovate[bot] Dec 10, 2023
ff90dd3
chore(deps): update dependency @babel/core to v7.23.5 (#327)
renovate[bot] Dec 10, 2023
81a5139
chore(deps): update dependency typescript to v5.3.3
renovate[bot] Dec 10, 2023
32dc1c7
chore(deps): update dependency @types/enzyme to v3.10.18 (#328)
renovate[bot] Dec 10, 2023
2602094
Merge pull request #332 from Magickbase/renovate/typescript-5.x
Keith-CY Dec 10, 2023
421c4c1
chore: update reviewers of renovate (#330)
Keith-CY Dec 11, 2023
a1fddf6
Merge branch 'develop' into develop
Keith-CY Dec 12, 2023
b36e1cb
refactor: refactor sign method to allow unidentified lock scripts (#2…
Keith-CY Dec 12, 2023
066845e
Merge branch 'develop' into alpha/skip-unidentified-inputs
Keith-CY Dec 12, 2023
a156dca
Merge branch 'develop' of https://github.com/nervosnetwork/neuron int…
Keith-CY Dec 14, 2023
56096fe
Merge branch 'develop' into alpha/skip-unidentified-inputs
Keith-CY Dec 15, 2023
c0f99d3
Merge branch 'develop' of https://github.com/nervosnetwork/neuron int…
Keith-CY Dec 21, 2023
90b8faf
Merge branch 'develop' into alpha/skip-unidentified-inputs
Keith-CY Dec 25, 2023
7950d0e
Merge branch 'develop' into alpha/skip-unidentified-inputs
Keith-CY Dec 26, 2023
a8d4660
Merge branch 'develop' into alpha/skip-unidentified-inputs
Keith-CY Jan 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/neuron-wallet/src/controllers/anyone-can-pay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export interface SendAnyoneCanPayTxParams {
walletID: string
tx: Transaction
password: string
skipLastInputs?: boolean
skipLastInput?: boolean
}

export default class AnyoneCanPayController {
Expand Down Expand Up @@ -64,7 +64,7 @@ export default class AnyoneCanPayController {
params.walletID,
txModel,
params.password,
params?.skipLastInputs ?? true,
params?.skipLastInput ?? true,
skipSign
)

Expand Down
20 changes: 13 additions & 7 deletions packages/neuron-wallet/src/controllers/offline-sign.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,13 +94,19 @@ export default class OfflineSignController {
context
)
} else {
tx = await new TransactionSender().sign(
walletID,
Transaction.fromObject(transaction),
password,
type === SignType.SendSUDT,
context
)
tx = await new TransactionSender()
.sign(walletID, Transaction.fromObject(transaction), password, type === SignType.SendSUDT, context)
.then(({ tx: t, metadata }) => {
// TODO: maybe unidentified inputs can be skipped in offline sign
if (metadata.locks.skipped.size) {
throw new Error(
`Fail to sign transaction, following lock scripts cannot be identified: ${[
...metadata.locks.skipped.values(),
]}`
)
}
return t
})
}

const signer = OfflineSign.fromJSON({
Expand Down
24 changes: 21 additions & 3 deletions packages/neuron-wallet/src/services/hardware/hardware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ export abstract class Hardware {
walletID: string,
tx: Transaction,
txHash: string,
skipLastInputs: boolean = true,
skipLastInput: boolean = true,
context?: RPC.RawTransaction[]
) {
const wallet = WalletService.getInstance().get(walletID)
const addressInfos = await AddressService.getAddressesByWalletId(walletID)
const witnessSigningEntries = tx.inputs.slice(0, skipLastInputs ? -1 : tx.inputs.length).map((input, index) => {
const witnessSigningEntries = tx.inputs.slice(0, skipLastInput ? -1 : tx.inputs.length).map((input, index) => {
const lockArgs: string = input.lock!.args!
const wit: WitnessArgs | string = tx.witnesses[index]
const witnessArgs: WitnessArgs = wit instanceof WitnessArgs ? wit : WitnessArgs.generateEmpty()
Expand Down Expand Up @@ -67,6 +67,13 @@ export abstract class Hardware {

const lockHashes = new Set(witnessSigningEntries.map(w => w.lockHash))

const metadata = {
locks: {
skipped: new Set<string>(),
},
skipLastInput,
}

for (const [index, lockHash] of [...lockHashes].entries()) {
DeviceSignIndexSubject.next(index)
const witnessesArgs = witnessSigningEntries.filter(w => w.lockHash === lockHash)
Expand All @@ -75,6 +82,17 @@ export abstract class Hardware {

const path = findPath(witnessesArgs[0].lockArgs)

if (!path) {
metadata.locks.skipped.add(lockHash)
witnessSigningEntries.forEach((entry, idx) => {
if (entry.lockHash === lockHash) {
const rawWitness = tx.witnesses[idx]
entry.witness = typeof rawWitness === 'string' ? rawWitness : serializeWitnessArgs(rawWitness)
}
})
continue
}

if (isMultisig) {
const serializedWitnesses = witnessesArgs.map(value => {
const args = value.witnessArgs
Expand Down Expand Up @@ -131,7 +149,7 @@ export abstract class Hardware {
tx.witnesses = witnessSigningEntries.map(w => w.witness || '0x')
tx.hash = txHash

return tx
return { tx, metadata }
}

public abstract getPublicKey(path: string): Promise<PublicKey>
Expand Down
59 changes: 42 additions & 17 deletions packages/neuron-wallet/src/services/transaction-sender.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,21 @@ export default class TransactionSender {
walletID: string = '',
transaction: Transaction,
password: string = '',
skipLastInputs: boolean = true,
skipLastInput: boolean = true,
skipSign = false
) {
const tx = skipSign
? Transaction.fromObject(transaction)
: await this.sign(walletID, transaction, password, skipLastInputs)
: await this.sign(walletID, transaction, password, skipLastInput).then(({ tx, metadata }) => {
if (metadata.locks.skipped.size) {
throw new Error(
`Fail to send transaction, following lock scripts cannot be identified: ${[
...metadata.locks.skipped.values(),
]} `
)
}
return tx
})

return this.broadcastTx(walletID, tx)
}
Expand Down Expand Up @@ -109,12 +118,13 @@ export default class TransactionSender {
walletID: string = '',
transaction: Transaction,
password: string = '',
skipLastInputs: boolean = true,
skipLastInput: boolean = true,
context?: RPC.RawTransaction[]
) {
const wallet = this.walletService.get(walletID)
const tx = Transaction.fromObject(transaction)
const txHash: string = tx.computeHash()

if (wallet.isHardware()) {
let device = HardwareWalletService.getInstance().getCurrent()
if (!device) {
Expand All @@ -124,7 +134,7 @@ export default class TransactionSender {
await device.connect()
}
try {
return await device.signTx(walletID, tx, txHash, skipLastInputs, context)
return await device.signTx(walletID, tx, txHash, skipLastInput, context)
} catch (err) {
if (err instanceof TypeError) {
throw err
Expand All @@ -151,23 +161,19 @@ export default class TransactionSender {
const findPrivateKey = (args: string) => {
let path: string | undefined
if (args.length === TransactionSender.MULTI_SIGN_ARGS_LENGTH) {
path = multiSignBlake160s.find(i => args.slice(0, 42) === i.multiSignBlake160)!.path
path = multiSignBlake160s.find(i => args.slice(0, 42) === i.multiSignBlake160)?.path
} else if (args.length === 42) {
path = addressInfos.find(i => i.blake160 === args)!.path
path = addressInfos.find(i => i.blake160 === args)?.path
} else {
const addressInfo = AssetAccountInfo.findSignPathForCheque(addressInfos, args)
path = addressInfo?.path
}

const pathAndPrivateKey = pathAndPrivateKeys.find(p => p.path === path)
if (!pathAndPrivateKey) {
throw new Error('no private key found')
}
return pathAndPrivateKey.privateKey
return pathAndPrivateKeys.find(p => p.path === path)?.privateKey
}

const witnessSigningEntries: SignInfo[] = tx.inputs
.slice(0, skipLastInputs ? -1 : tx.inputs.length)
.slice(0, skipLastInput ? -1 : tx.inputs.length)
.map((input: Input, index: number) => {
const lockArgs: string = input.lock!.args!
const wit: WitnessArgs | string = tx.witnesses[index]
Expand All @@ -183,13 +189,32 @@ export default class TransactionSender {

const lockHashes = new Set(witnessSigningEntries.map(w => w.lockHash))

const metadata = {
locks: {
skipped: new Set<string>(),
},
skipLastInput,
}

for (const lockHash of lockHashes) {
const witnessesArgs = witnessSigningEntries.filter(w => w.lockHash === lockHash)
// A 65-byte empty signature used as placeholder
witnessesArgs[0].witnessArgs.setEmptyLock()

const privateKey = findPrivateKey(witnessesArgs[0].lockArgs)

if (!privateKey) {
metadata.locks.skipped.add(lockHash)
witnessSigningEntries.forEach((entry, idx) => {
if (entry.lockHash === lockHash) {
const rawWitness = tx.witnesses[idx]
entry.witness = typeof rawWitness === 'string' ? rawWitness : serializeWitnessArgs(rawWitness)
}
})
continue
}

// A 65-byte empty signature used as placeholder
witnessesArgs[0].witnessArgs.setEmptyLock()

const serializedWitnesses: (WitnessArgs | string)[] = witnessesArgs.map((value: SignInfo, index: number) => {
const args = value.witnessArgs
if (index === 0) {
Expand Down Expand Up @@ -238,7 +263,7 @@ export default class TransactionSender {
tx.witnesses = witnessSigningEntries.map(w => w.witness)
tx.hash = txHash

return tx
return { tx, metadata }
}

public async signMultisig(
Expand Down Expand Up @@ -266,7 +291,7 @@ export default class TransactionSender {
} else {
pathAndPrivateKeys = this.getPrivateKeys(wallet, paths, password)
}
const findPrivateKeyAndBlake160 = (argsList: string[], signedBlake160s?: string[]) => {
const findPrivateKeyAndBlake160 = (argsList: string[], signedBlake160s?: string[]): [string, string] => {
let path: string | undefined
let matchArgs: string | undefined
argsList.some(args => {
Expand All @@ -284,7 +309,7 @@ export default class TransactionSender {
}
return !!path
})
if (!path) {
if (!path || !matchArgs) {
throw new NoMatchAddressForSign()
}
if (!pathAndPrivateKeys) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ describe('anyone-can-pay-controller', () => {
walletID: 'string',
tx: new Transaction('', [], [], [], [], []),
password: 'string',
skipLastInputs: false,
skipLastInput: false,
}
it('throw exception', async () => {
sendTxMock.mockResolvedValueOnce(undefined)
Expand Down
12 changes: 9 additions & 3 deletions packages/neuron-wallet/tests/controllers/offline-sign.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ describe('OfflineSignController', () => {
},
}

const mockTransactionMetadata = { locks: { skipped: new Set() } }

const mockTxInstance = {
toSDKRawTransaction() {
return mockTransaction
Expand Down Expand Up @@ -347,7 +349,9 @@ describe('OfflineSignController', () => {
filePath: 'filePath.json',
})

stubbedTransactionSenderSign.mockReturnValue(mockTransaction)
stubbedTransactionSenderSign.mockReturnValue(
Promise.resolve({ tx: mockTransaction, metadata: mockTransactionMetadata })
)
})

it('sign status should change to `signed`', async () => {
Expand Down Expand Up @@ -413,7 +417,9 @@ describe('OfflineSignController', () => {
filePath: 'filePath.json',
})

stubbedTransactionSenderSign.mockReturnValue(mockTransaction)
stubbedTransactionSenderSign.mockReturnValue(
Promise.resolve({ tx: mockTransaction, metadata: mockTransactionMetadata })
)
})

it('should signed', async () => {
Expand Down Expand Up @@ -486,7 +492,7 @@ describe('OfflineSignController', () => {
describe('signAndBroadcastTransaction', () => {
beforeEach(() => {
resetMocks()
signMultisigMock.mockReturnValue(mockTransaction)
signMultisigMock.mockReturnValue({ tx: mockTransaction, metadata: mockTransactionMetadata })
})
it('throw exception', async () => {
getMultisigStatusMock.mockReturnValueOnce(SignStatus.PartiallySigned)
Expand Down
Loading