Skip to content

Signing Bug fixes #741

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.

Sign up for GitHub

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

Merged
derekpierre merged 5 commits into from
Nov 20, 2025
Merged

Signing Bug fixes #741

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5663ca4
Properly sort signatures based on signer addresses when aggregating s…
derekpierre Nov 20, 2025
7a5dd56
Use `accountGasLimits` and not `accountGasLimit` as property on Packe…
derekpierre Nov 20, 2025
fffefe2
Allow optional signature property on PackedUserOperationToSign.
derekpierre Nov 20, 2025
d09fad7
Expose `PackedUserOperationToSign` and `UserOperationToSign` from wit…
derekpierre Nov 20, 2025
838cf34
Fix sorting of signatures when aggregating signatures.
derekpierre Nov 20, 2025
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
5 changes: 3 additions & 2 deletions packages/shared/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,11 @@ export type PackedUserOperationToSign = {
nonce: bigint | number;
initCode: `0x${string}` | Uint8Array;
callData: `0x${string}` | Uint8Array;
accountGasLimit: `0x${string}` | Uint8Array;
accountGasLimits: `0x${string}` | Uint8Array;
preVerificationGas: bigint | number;
gasFees: `0x${string}` | Uint8Array;
paymasterAndData: `0x${string}` | Uint8Array;
signature?: `0x${string}` | Uint8Array | undefined;
};

function getBigIntValue(value: bigint | number): bigint {
Expand Down Expand Up @@ -101,7 +102,7 @@ export function toCorePackedUserOperation(
getBigIntValue(packedUserOperation.nonce),
getUint8ArrayValue(packedUserOperation.initCode),
getUint8ArrayValue(packedUserOperation.callData),
getUint8ArrayValue(packedUserOperation.accountGasLimit),
getUint8ArrayValue(packedUserOperation.accountGasLimits),
getBigIntValue(packedUserOperation.preVerificationGas),
getUint8ArrayValue(packedUserOperation.gasFees),
getUint8ArrayValue(packedUserOperation.paymasterAndData),
Expand Down
2 changes: 1 addition & 1 deletion packages/shared/test/porter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ describe('PorterClient Signing', () => {
BigInt(123), // nonce
fromHexString('0xabc'), // initCode
fromHexString('0xdef'), // callData
fromHexString('0x01020304'), // accountGasLimit
fromHexString('0x01020304'), // accountGasLimits
BigInt(101112), // preVerificationGas
fromHexString('0x05060708'), // gasFees
fromHexString('0x090a0b0c'), // paymasterAndData
Expand Down
2 changes: 2 additions & 0 deletions packages/taco/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
export { DkgPublicKey, ThresholdMessageKit } from '@nucypher/nucypher-core';
export {
Domain,
PackedUserOperationToSign,
UserOperationToSign,
domains,
fromBytes,
getPorterUris,
Expand Down
34 changes: 22 additions & 12 deletions packages/taco/src/sign.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,20 +51,30 @@
};

function aggregateSignatures(
signaturesByAddress: {
[checksumAddress: string]: TacoSignature;
},
signatures: TacoSignature[],
threshold: number,
): string {
// Aggregate hex signatures by concatenating them; being careful to remove the '0x' prefix from each signature except the first one.
const signatures = Object.values(signaturesByAddress)
.map((sig) => sig.signature)
.slice(0, threshold);
if (signatures.length === 1) {
return signatures[0];
// Aggregate hex signatures by concatenating them; being careful to sort
// and remove the '0x' prefix from each signature except the first one.

// sort by signer address
const sortedSignatures = [...signatures]
.sort((a, b) =>
a.signerAddress
.toLowerCase()
.localeCompare(b.signerAddress.toLowerCase()),
)
.map((sig) => sig.signature);

const thresholdSignatures = sortedSignatures.slice(0, threshold);
if (thresholdSignatures.length === 1) {
return thresholdSignatures[0];
}

// Concatenate signatures
const allBytes = signatures.flatMap((hex) => Array.from(fromHexString(hex)));
const allBytes = thresholdSignatures.flatMap((sig) =>
Array.from(fromHexString(sig)),
);
return `0x${toHexString(new Uint8Array(allBytes))}`;
}

Expand Down Expand Up @@ -187,7 +197,7 @@
);
if (Object.keys(encryptedResponses).length < threshold) {
// not enough signatures returned
throw new Error(ERR_INSUFFICIENT_SIGNATURES(errors));

Check failure on line 200 in packages/taco/src/sign.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test on Node 18.x and ubuntu-latest

integration-test/sign.test.ts > TACo Sign Integration Test > should sign a user operation 

Error: Threshold of signatures not met; TACo signing failed with errors: {"0xb15d5A4e2be34f4bE154A1b08a94Ab920FfD8A41":"(<class 'nucypher.network.concurrency.SigningRequestClient.SigningRequestFailed'>, SigningRequestFailed('Node 0xb15d5A4e2be34f4bE154A1b08a94Ab920FfD8A41 raised '), <traceback object at 0x7070e72ee840>)","0x210eeAC07542F815ebB6FD6689637D8cA2689392":"(<class 'nucypher.network.concurrency.SigningRequestClient.SigningRequestFailed'>, SigningRequestFailed('Node 0x210eeAC07542F815ebB6FD6689637D8cA2689392 raised '), <traceback object at 0x7070e6975b40>)","0x48C8039c32F4c6f5cb206A5911C8Ae814929C16B":"(<class 'nucypher.network.concurrency.SigningRequestClient.SigningRequestFailed'>, SigningRequestFailed('Node 0x48C8039c32F4c6f5cb206A5911C8Ae814929C16B raised '), <traceback object at 0x7070e72f1600>)"}
 ❯ signUserOp src/sign.ts:200:11
 ❯ integration-test/sign.test.ts:70:26

Check failure on line 200 in packages/taco/src/sign.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test on Node 20.x and ubuntu-latest

integration-test/sign.test.ts > TACo Sign Integration Test > should sign a user operation 

Error: Threshold of signatures not met; TACo signing failed with errors: {"0xb15d5A4e2be34f4bE154A1b08a94Ab920FfD8A41":"(<class 'nucypher.network.concurrency.SigningRequestClient.SigningRequestFailed'>, SigningRequestFailed('Node 0xb15d5A4e2be34f4bE154A1b08a94Ab920FfD8A41 raised '), <traceback object at 0x76c054cb42c0>)","0x210eeAC07542F815ebB6FD6689637D8cA2689392":"(<class 'nucypher.network.concurrency.SigningRequestClient.SigningRequestFailed'>, SigningRequestFailed('Node 0x210eeAC07542F815ebB6FD6689637D8cA2689392 raised '), <traceback object at 0x76c054dbb300>)","0x48C8039c32F4c6f5cb206A5911C8Ae814929C16B":"(<class 'nucypher.network.concurrency.SigningRequestClient.SigningRequestFailed'>, SigningRequestFailed('Node 0x48C8039c32F4c6f5cb206A5911C8Ae814929C16B raised '), <traceback object at 0x76c054d0bec0>)"}
 ❯ signUserOp src/sign.ts:200:11
 ❯ integration-test/sign.test.ts:70:26

Check failure on line 200 in packages/taco/src/sign.ts

View workflow job for this annotation

GitHub Actions / Build, lint, and test on Node 22.x and ubuntu-latest

integration-test/sign.test.ts > TACo Sign Integration Test > should sign a user operation 

Error: Threshold of signatures not met; TACo signing failed with errors: {"0xb15d5A4e2be34f4bE154A1b08a94Ab920FfD8A41":"(<class 'nucypher.network.concurrency.SigningRequestClient.SigningRequestFailed'>, SigningRequestFailed('Node 0xb15d5A4e2be34f4bE154A1b08a94Ab920FfD8A41 raised '), <traceback object at 0x7070e6924940>)","0x210eeAC07542F815ebB6FD6689637D8cA2689392":"(<class 'nucypher.network.concurrency.SigningRequestClient.SigningRequestFailed'>, SigningRequestFailed('Node 0x210eeAC07542F815ebB6FD6689637D8cA2689392 raised '), <traceback object at 0x7070e6925840>)","0x48C8039c32F4c6f5cb206A5911C8Ae814929C16B":"(<class 'nucypher.network.concurrency.SigningRequestClient.SigningRequestFailed'>, SigningRequestFailed('Node 0x48C8039c32F4c6f5cb206A5911C8Ae814929C16B raised '), <traceback object at 0x7070e6935c80>)"}
 ❯ signUserOp src/sign.ts:200:11
 ❯ integration-test/sign.test.ts:70:26
}

const signaturesToAggregate = collectSignatures(
Expand All @@ -197,7 +207,7 @@
);

const aggregatedSignature = aggregateSignatures(
signaturesToAggregate,
Object.values(signaturesToAggregate),
threshold,
);

Expand Down Expand Up @@ -300,7 +310,7 @@

// Insufficient signatures for a message hash to meet the threshold
if (!signaturesToAggregate) {
//we have multiple hashes, which means we have mismatched hashes from different nodes
// we have multiple hashes, which means we have mismatched hashes from different nodes
// we don't really expect this to happen (other than some malicious nodes)
console.error(
'Porter returned mismatched message hashes:',
Expand Down
33 changes: 17 additions & 16 deletions packages/taco/test/taco-sign.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ function checkPackedUserOpEquality(
: fromHexString(op1.callData);
expect(callData).toEqual(op2.callData);

const accountGasLimit =
op1.accountGasLimit instanceof Uint8Array
? op1.accountGasLimit
: fromHexString(op1.accountGasLimit);
expect(accountGasLimit).toEqual(op2.accountGasLimits);
const accountGasLimits =
op1.accountGasLimits instanceof Uint8Array
? op1.accountGasLimits
: fromHexString(op1.accountGasLimits);
expect(accountGasLimits).toEqual(op2.accountGasLimits);

expect(toBigInt(op1.preVerificationGas)).toEqual(op2.preVerificationGas);

Expand Down Expand Up @@ -290,7 +290,7 @@ describe('TACo Signing', () => {
nonce: BigInt(123),
initCode: fromHexString('0xabc'),
callData: fromHexString('0xdef'),
accountGasLimit: fromHexString('0x01020304'),
accountGasLimits: fromHexString('0x01020304'),
preVerificationGas: BigInt(101112),
gasFees: fromHexString('0x05060708'),
paymasterAndData: fromHexString('0x090a0b0c'),
Expand All @@ -307,7 +307,7 @@ describe('TACo Signing', () => {
nonce: 1,
initCode: '0xabc',
callData: '0xdef',
accountGasLimit: '0x01020304',
accountGasLimits: '0x01020304',
preVerificationGas: 4096,
gasFees: '0x05060708',
paymasterAndData: '0x090a0b0c',
Expand All @@ -333,7 +333,7 @@ describe('TACo Signing', () => {
nonce: BigInt(1),
initCode: fromHexString('0xabc'),
callData: fromHexString('0xdef'),
accountGasLimit: fromHexString('0x01020304'),
accountGasLimits: fromHexString('0x01020304'),
preVerificationGas: BigInt(101112),
gasFees: fromHexString('0x05060708'),
paymasterAndData: fromHexString('0x090a0b0c'),
Expand All @@ -357,24 +357,25 @@ describe('TACo Signing', () => {
userOp: UserOperationToSign | PackedUserOperationToSign,
) => {
const encryptedResponses = {
'0xnode1': new SignatureResponse(
signersInfo['0xnode1'].signerAddress,
// return out of order to ensure sorting works correctly
'0xnode2': new SignatureResponse(
signersInfo['0xnode2'].signerAddress,
fromHexString('0xa1'),
fromHexString('0xdead'),
fromHexString('0xbeef'),
0,
).encrypt(
requesterSk.deriveSharedSecret(
signersInfo['0xnode1'].signingRequestStaticKey,
signersInfo['0xnode2'].signingRequestStaticKey,
),
),
'0xnode2': new SignatureResponse(
signersInfo['0xnode2'].signerAddress,
'0xnode1': new SignatureResponse(
signersInfo['0xnode1'].signerAddress,
fromHexString('0xa1'),
fromHexString('0xbeef'),
fromHexString('0xdead'),
0,
).encrypt(
requesterSk.deriveSharedSecret(
signersInfo['0xnode2'].signingRequestStaticKey,
signersInfo['0xnode1'].signingRequestStaticKey,
),
),
};
Expand Down
Loading