Skip to content

Commit

Permalink
refactor: refactor sign method to allow unidentified lock scripts
Browse files Browse the repository at this point in the history 
The sign method is refactored to allow unidentified lock scripts
while skipped lock scripts are prevented outside the sign method

Notice that skipped lock scripts validation is not adopted when
offline-sign a multisig transaction because it's explictly thrown
when no private key is found. It may be intended.
  • Loading branch information
@Keith-CY
Keith-CY committed Dec 8, 2023
1 parent ea19420 commit 7abc5ba
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 32 deletions.
20 changes: 13 additions & 7 deletions packages/neuron-wallet/src/controllers/offline-sign.ts
View file
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
20 changes: 19 additions & 1 deletion packages/neuron-wallet/src/services/hardware/hardware.ts
View file
Original file line number Diff line number Diff line change
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
51 changes: 38 additions & 13 deletions packages/neuron-wallet/src/services/transaction-sender.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
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
65 changes: 54 additions & 11 deletions packages/neuron-wallet/tests/services/tx/transaction-sender.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -343,7 +344,7 @@ describe('TransactionSender Test', () => {

describe('#sign', () => {
describe('single sign', () => {
const tx = Transaction.fromObject({
const tx = {
version: '0x0',
cellDeps: [
CellDep.fromObject({
Expand Down Expand Up @@ -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)
})
})

Expand Down Expand Up @@ -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)
})
})

Expand Down Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down

1 comment on commit 7abc5ba

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Packaging for test is done in 7138028753

Please sign in to comment.