Repository navigation
Conversation
sumPubKeys and _sumPrivkeys aborted as soon as tiny-secp256k1 returned null for the point at infinity / zero scalar, so [A, -A, A] failed while [A, A, -A] succeeded. Continue through intermediate identity and only fail if the final sum is identity. Fixes BlueWallet#30
GladosBlueWallet
left a comment
GladosBlueWallet
left a comment
There was a problem hiding this comment.
Bugfix PR for #30: intermediate ECC identity was aborting sumPubKeys/_sumPrivkeys early. Core scalar/point logic looks sound for compressed keys, but the test chamber is half-built — _sumPrivkeys/createTransaction and final-zero edges are untested, and the infinity-resume path still faceplants on x-only successors.
| console.log("1000 tweak mults took", (end - start) / 1000, "sec"); | ||
| }); | ||
|
|
||
| it("sumPubKeys is order-independent when an intermediate sum is the point at infinity", () => { |
There was a problem hiding this comment.
[HIGH] Your test plan promised both key orders for pubkey and privkey sums. I only see sumPubKeys. _sumPrivkeys and the createTransaction [A,-A,A] failure from #30 remain unobserved — excellent science, if you enjoy not knowing whether half the fix works.
| console.log("1000 tweak mults took", (end - start) / 1000, "sec"); | ||
| }); | ||
|
|
||
| it("sumPubKeys is order-independent when an intermediate sum is the point at infinity", () => { |
There was a problem hiding this comment.
[HIGH] You patched the 'stop at intermediate zero' bug but never verify the actual failure boundary: [A,-A] must stay null, and three keys that fully cancel must still throw in _sumPrivkeys. Edge cases are the whole experiment; skipping them is how subjects lie about passing.
| let atIdentity = false; | ||
| for (let i = 1; i < pubkeys.length; i++) { | ||
| if (atIdentity) { | ||
| result = pubkeys[i]; |
There was a problem hiding this comment.
[MEDIUM] After infinity you blindly adopt the next pubkey as result. getPubkeysFromTransactionInputs feeds x-only 32-byte keys; the very next pointAdd at 321 demands compressed/uncompressed and throws. Compressed cancel-then-x-only still detonates — you fixed one ordering trap and left a mixed-encoding trapdoor.
Overtorment
left a comment
Overtorment
left a comment
There was a problem hiding this comment.
LGTM, i think its mergeable as is, but let me know if you want to address glados remarks
|
merged in #33 |
3 participants
Treat intermediate identity as identity in pubkey and privkey sums.
Reported by @shuv-amp.
Fixes #30
sumPubKeys and _sumPrivkeys aborted as soon as an intermediate sum was the ECC identity.
Continue through intermediate identity; only fail if the final sum is identity.
Test plan