Skip to content

DeriveInterpolatingValue uses integer division instead of modular inverse #9

Description

@rdubois-crypto

DeriveInterpolatingValue uses integer division instead of modular inverse this leads to wrong Lagrange coefficients for most quorums (for instance try 1,3 corum).

The bug is invisible for quorums whose denominator happens to be 1 — which includes {1,2}, the quorum used by the existing tests. It only surfaces on other signer subsets.

Reproduction
tsfrost.deriveInterpolatingValue([1n, 3n], 1n) // → 1n (expected 3·2⁻¹ mod L)
frost.deriveInterpolatingValue([1n, 3n], 3n) // → 0n (expected −1·2⁻¹ mod L)

Suggested fix
Reduce mod the subgroup order at each step and invert with extended Euclid:
tsderiveInterpolatingValue(L: bigint[], x_i: bigint) {
const found = L.find(a => a === x_i)
if (!found) throw new Error('invalid parameters')
let num = 1n
let dom = 1n
for (const x_j of L) {
if (x_j === x_i) continue
num = this.modOrder(num * x_j)
dom = this.modOrder(dom * (x_j - x_i))
}
return this.modOrder(num * this.modInverse(dom))
}

private modInverse(a: bigint): bigint {
const m = this.order
let [r, r1] = [this.modOrder(a), m]
let [x, x1] = [1n, 0n]
while (r1 !== 0n) {
const q = r / r1
;[r, r1] = [r1, r - q * r1]
;[x, x1] = [x1, x - q * x1]
}
if (r !== 1n) throw new Error('not invertible')
return this.modOrder(x)
}

(patch coming soon)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions