diff --git a/packages/neuron-wallet/src/controllers/offline-sign.ts b/packages/neuron-wallet/src/controllers/offline-sign.ts index 1ca40ba019..c31bb8865b 100644 --- a/packages/neuron-wallet/src/controllers/offline-sign.ts +++ b/packages/neuron-wallet/src/controllers/offline-sign.ts @@ -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({ diff --git a/packages/neuron-wallet/src/services/hardware/hardware.ts b/packages/neuron-wallet/src/services/hardware/hardware.ts index 417d8d6dcb..2c5801a4cc 100644 --- a/packages/neuron-wallet/src/services/hardware/hardware.ts +++ b/packages/neuron-wallet/src/services/hardware/hardware.ts @@ -67,6 +67,13 @@ export abstract class Hardware { const lockHashes = new Set(witnessSigningEntries.map(w => w.lockHash)) + const metadata = { + locks: { + skipped: new Set(), + }, + skipLastInput, + } + for (const [index, lockHash] of [...lockHashes].entries()) { DeviceSignIndexSubject.next(index) const witnessesArgs = witnessSigningEntries.filter(w => w.lockHash === lockHash) @@ -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 @@ -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 diff --git a/packages/neuron-wallet/src/services/transaction-sender.ts b/packages/neuron-wallet/src/services/transaction-sender.ts index ff2a551f26..614f179351 100644 --- a/packages/neuron-wallet/src/services/transaction-sender.ts +++ b/packages/neuron-wallet/src/services/transaction-sender.ts @@ -72,7 +72,16 @@ export default class TransactionSender { ) { const tx = skipSign ? Transaction.fromObject(transaction) - : await this.sign(walletID, transaction, password, skipLastInput) + : 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) } @@ -115,6 +124,7 @@ export default class TransactionSender { 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) { @@ -151,19 +161,15 @@ 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 @@ -183,13 +189,32 @@ export default class TransactionSender { const lockHashes = new Set(witnessSigningEntries.map(w => w.lockHash)) + const metadata = { + locks: { + skipped: new Set(), + }, + 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) { @@ -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( @@ -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 => { @@ -284,7 +309,7 @@ export default class TransactionSender { } return !!path }) - if (!path) { + if (!path || !matchArgs) { throw new NoMatchAddressForSign() } if (!pathAndPrivateKeys) { diff --git a/packages/neuron-wallet/tests/services/tx/transaction-sender.test.ts b/packages/neuron-wallet/tests/services/tx/transaction-sender.test.ts index d52372dc3a..62974c8b29 100644 --- a/packages/neuron-wallet/tests/services/tx/transaction-sender.test.ts +++ b/packages/neuron-wallet/tests/services/tx/transaction-sender.test.ts @@ -204,6 +204,7 @@ import MultisigConfigModel from '../../../src/models/multisig-config' import Multisig from '../../../src/models/multisig' import { addressToScript } from '../../../src/utils/scriptAndAddress' import { serializeWitnessArgs } from '../../../src/utils/serialization' +import { signWitnesses } from '../../../src/utils/signWitnesses' const fakeScript = new Script( '0x9bd7e06f3ecf4be0f2fcd2188b23f1b9fcc88e5d4b65a8637b17723bbda3cce8', @@ -343,7 +344,7 @@ describe('TransactionSender Test', () => { describe('#sign', () => { describe('single sign', () => { - const tx = Transaction.fromObject({ + const tx = { version: '0x0', cellDeps: [ CellDep.fromObject({ @@ -393,13 +394,49 @@ describe('TransactionSender Test', () => { witnesses: [ '0x55000000100000005500000055000000410000003965f54cc684d35d886358ad57214e5f4a5fd13ecc7aba67950495b9be7740267a1d6bb14f1c215e3bc926f9655648b75e173ce6f5fd1e60218383b45503c30301', ], - hash: '0x230ab250ee0ae681e88e462102e5c01a9994ac82bf0effbfb58d6c11a86579f1', - }) + } it('success', async () => { - const ntx = await transactionSender.sign(fakeWallet.id, tx, '1234', false) + const res = await transactionSender.sign(fakeWallet.id, Transaction.fromObject(tx), '1234', false) + expect(res.tx.witnesses[0]).toEqual(tx.witnesses[0]) + expect(res.metadata.locks.skipped.size).toEqual(0) + expect(res.metadata.skipLastInput).toEqual(false) + }) + + it('sign with unidentified lock scripts skipped', async () => { + const FIXTURE = { + lock: Input.fromObject({ + previousOutput: OutPoint.fromObject({ + txHash: '0x1879851943fa686af29bed5c95acd566d0244e7b3ca89cf7c435622a5a5b4cb3', + index: '0x0', + }), + since: '0x0', + lock: Script.fromObject({ + args: '0x0000000000000000000000000000000000000000', + codeHash: '0x9bd7e06f3ecf4be0f2fcd2188b23f1b9fcc88e5d4b65a8637b17723bbda3cce8', + hashType: 'type' as ScriptHashType, + }), + }), + witness: new WitnessArgs('0x01'), + } + + const txObj = Transaction.fromObject({ + ...tx, + inputs: [...tx.inputs, FIXTURE.lock], + witnesses: [new WitnessArgs('0x00'), FIXTURE.witness], + }) + + const signedWitnesses = signWitnesses({ + witnesses: txObj.witnesses.slice(0, -1), + transactionHash: txObj.computeHash(), + privateKey: pathAndPrivateKey.privateKey, + }) - expect(ntx.witnesses[0]).toEqual(tx.witnesses[0]) + const res = await transactionSender.sign(fakeWallet.id, txObj, '1234', false) + expect(res.tx.witnesses.slice(0, -1)).toEqual(signedWitnesses) + expect(res.tx.witnesses.slice(-1)).toEqual([serializeWitnessArgs(FIXTURE.witness)]) + expect([...res.metadata.locks.skipped.values()]).toEqual([FIXTURE.lock.lockHash]) + expect(res.metadata.skipLastInput).toEqual(false) }) }) @@ -453,9 +490,11 @@ describe('TransactionSender Test', () => { it('success', async () => { // @ts-ignore: Private method - const ntx = await transactionSender.sign(fakeWallet.id, tx, '1234', false) + const res = await transactionSender.sign(fakeWallet.id, tx, '1234', false) - expect(ntx.witnesses[0]).toEqual(expectedWitness[0]) + expect(res.tx.witnesses[0]).toEqual(expectedWitness[0]) + expect(res.metadata.locks.skipped.size).toEqual(0) + expect(res.metadata.skipLastInput).toEqual(false) }) }) @@ -491,9 +530,11 @@ describe('TransactionSender Test', () => { }) it('success', async () => { // @ts-ignore: Private method - const ntx = await transactionSender.sign(fakeWallet.id, tx, '1234', false) + const res = await transactionSender.sign(fakeWallet.id, tx, '1234', false) - expect(ntx.witnesses[0]).toEqual(tx.witnesses[0]) + expect(res.tx.witnesses[0]).toEqual(tx.witnesses[0]) + expect(res.metadata.locks.skipped.size).toEqual(0) + expect(res.metadata.skipLastInput).toEqual(false) }) }) describe('when not matched receiver lock hash', () => { @@ -546,9 +587,11 @@ describe('TransactionSender Test', () => { tx.inputs[0].lock = chequeLock }) it('success', async () => { - const ntx = await transactionSender.sign(fakeWallet.id, tx, '1234', false) + const res = await transactionSender.sign(fakeWallet.id, tx, '1234', false) - expect(ntx.witnesses[0]).toEqual(tx.witnesses[0]) + expect(res.tx.witnesses[0]).toEqual(tx.witnesses[0]) + expect(res.metadata.locks.skipped.size).toEqual(0) + expect(res.metadata.skipLastInput).toEqual(false) }) }) describe('when not matched sender lock hash', () => {