diff --git a/packages/neuron-wallet/src/controllers/anyone-can-pay.ts b/packages/neuron-wallet/src/controllers/anyone-can-pay.ts index b74267ea19..6ce97115b5 100644 --- a/packages/neuron-wallet/src/controllers/anyone-can-pay.ts +++ b/packages/neuron-wallet/src/controllers/anyone-can-pay.ts @@ -21,7 +21,7 @@ export interface SendAnyoneCanPayTxParams { walletID: string tx: Transaction password: string - skipLastInputs?: boolean + skipLastInput?: boolean } export default class AnyoneCanPayController { @@ -64,7 +64,7 @@ export default class AnyoneCanPayController { params.walletID, txModel, params.password, - params?.skipLastInputs ?? true, + params?.skipLastInput ?? true, skipSign ) 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 20af74e74c..2c5801a4cc 100644 --- a/packages/neuron-wallet/src/services/hardware/hardware.ts +++ b/packages/neuron-wallet/src/services/hardware/hardware.ts @@ -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() @@ -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 9820c9c0e7..d26904aac4 100644 --- a/packages/neuron-wallet/src/services/transaction-sender.ts +++ b/packages/neuron-wallet/src/services/transaction-sender.ts @@ -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) } @@ -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) { @@ -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 @@ -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] @@ -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/controllers/anyone-can-pay.test.ts b/packages/neuron-wallet/tests/controllers/anyone-can-pay.test.ts index dd906cea2d..bee9d07b2e 100644 --- a/packages/neuron-wallet/tests/controllers/anyone-can-pay.test.ts +++ b/packages/neuron-wallet/tests/controllers/anyone-can-pay.test.ts @@ -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) diff --git a/packages/neuron-wallet/tests/controllers/offline-sign.test.ts b/packages/neuron-wallet/tests/controllers/offline-sign.test.ts index 7bd987a1c1..5d1d7bfd65 100644 --- a/packages/neuron-wallet/tests/controllers/offline-sign.test.ts +++ b/packages/neuron-wallet/tests/controllers/offline-sign.test.ts @@ -115,6 +115,8 @@ describe('OfflineSignController', () => { }, } + const mockTransactionMetadata = { locks: { skipped: new Set() } } + const mockTxInstance = { toSDKRawTransaction() { return mockTransaction @@ -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 () => { @@ -413,7 +417,9 @@ describe('OfflineSignController', () => { filePath: 'filePath.json', }) - stubbedTransactionSenderSign.mockReturnValue(mockTransaction) + stubbedTransactionSenderSign.mockReturnValue( + Promise.resolve({ tx: mockTransaction, metadata: mockTransactionMetadata }) + ) }) it('should signed', async () => { @@ -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) 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 f4ca99d4e1..3bbe0c9f5b 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', () => {