Skip to content
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.

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

Constant-time EC point multiplication (Montgomery ladder) implementation #325

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
130b977
feat(lean-imt): added `updateMany` method to package
ChinoCribioli Aug 28, 2024
6049ea4
feat(lean-imt): implemented some tests on lean-imt
ChinoCribioli Aug 28, 2024
e00a891
feat(lean-imt): added more precondition checks
ChinoCribioli Aug 29, 2024
3e9e54b
feat(lean-imt): finished testing on `updateMany` method
ChinoCribioli Aug 29, 2024
cd3156e
feat(lean-imt): added test to the case when passing repeated indices
ChinoCribioli Aug 31, 2024
06e941c
feat(lean-imt): added complexity documentation for `updateMany` method
ChinoCribioli Aug 31, 2024
4b51e44
feat(lean-imt): added test of several updates
ChinoCribioli Sep 1, 2024
29638d5
feat(lean-imt): added repeated indices check
ChinoCribioli Sep 1, 2024
880812c
feat(lean-imt): changed error message to be more accurate
ChinoCribioli Sep 1, 2024
681239e
feat(lean-imt): added complexity in terms only of n
ChinoCribioli Sep 1, 2024
1bfffd4
feat(lean-imt): changed documentation to add discussion in another issue
ChinoCribioli Sep 2, 2024
bd67b35
feat(lean-imt): fixed typo on documentation
ChinoCribioli Sep 2, 2024
c5e836d
Update packages/lean-imt/src/lean-imt.ts
ChinoCribioli Sep 9, 2024
6270ad5
Merge branch 'privacy-scaling-explorations:main' into main
ChinoCribioli Sep 9, 2024
fe22dac
perf(baby-jubjub): implemented montgomery ladder
ChinoCribioli Sep 10, 2024
6665e36
test(baby-jubjub): added more tests to curve basic operations
ChinoCribioli Sep 11, 2024
fc0f3b8
docs(baby-jubjub): documented montogmery ladder
ChinoCribioli Sep 11, 2024
8127d10
docs(baby-jubjub): fixed documentation
ChinoCribioli Sep 11, 2024
3c122c5
docs(baby-jubjub): added resource of Montgomery Ladder
ChinoCribioli Sep 12, 2024
56fcd9f
fix(baby-jubjub): replaced undetermined while with hardcoded for loop
ChinoCribioli Sep 12, 2024
b7dec5b
refactor(baby-jubjub): created and exported identity point
ChinoCribioli Sep 12, 2024
f0ad8e1
Update packages/baby-jubjub/src/baby-jubjub.ts
ChinoCribioli Sep 13, 2024
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
31 changes: 20 additions & 11 deletions packages/baby-jubjub/src/baby-jubjub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,25 +65,34 @@ export function addPoint(p1: Point<bigint>, p2: Point<bigint>): Point<bigint> {
/**
* Performs a scalar multiplication by starting from the 'base' point and 'adding'
* it to itself 'e' times.
* This algorithm is called 'Montgomery Ladder'. See {@link https://en.wikipedia.org/wiki/Elliptic_curve_point_multiplication#Montgomery_ladder}
* This works given the following invariant: At each step, R0 will be r_0*base where r_0 is the prefix of e
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest additionally referring to the named algorithm being used here (Montgomery Ladder) perhaps with a link to a description of the algorithm (Wikipedia or some other source).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure! Will do.

* written in binary and R1 will be (r_0+1)*base. In other words: at iteration i of the loop, r_0's binary
* representation will be the first i+1 most significant bits of e. If the upcoming bit is a 0, we just have to
* double R0 and add R0 to R1 to maintain the invariant. If it is a 1, we have to double R0 and add 1*base
* (or add R1, which is the same as (r_0+1)*base), and double R1 to maintain the invariant.
* @param base The base point used as a starting point.
* @param e A secret number representing the private key.
* @returns The resulting point representing the public key.
*/
export function mulPointEscalar(base: Point<bigint>, e: bigint): Point<bigint> {
let res: Point<bigint> = [Fr.e(BigInt(0)), Fr.e(BigInt(1))]
let rem: bigint = e
let exp: Point<bigint> = base

while (!scalar.isZero(rem)) {
if (scalar.isOdd(rem)) {
res = addPoint(res, exp)
e %= order
ChinoCribioli marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looking at this again, the original implementation doesn't make any assertions or modifications to this variable. This is an exponent, which is not necessarily a field element, so the reduction should be unnecessary.

I think if we remove this we have a pretty safe function. We just need to look more deeply at the performance hit and decide if it's worth it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

An exponent doesn't have to be a field element, but the curve is cyclic with its order, so a value greater than the order is redundant compared to the modular reduction of the same value (either leads to the same output). And the constant-ish time algorithm below works only if you can fix the number of bits in the input. If we reduce the modular reduction, then I think this function becomes incorrect for any value greater than the order. Modular reduction is not the same as cutting off high bits (because the order is not a power of 2).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed. As Andrew says, some tests break when you remove the reduction because you incorrectly handle the case where the exponent passed as input is greater than 254 bits.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah right because the loop count is hardcoded, i see


let R0: Point<bigint> = [0n, 1n]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see there's a unit test covering multiply by zero, which is good to confirm this still behaves the same way as before without the special-case.
As a side suggestion, it might be clearer to make this a public const id like is used in the unit tests, to make it clear that [0n, 1n] is the representation of the zero/id point in this curve.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like that. It is done.

let R1: Point<bigint> = base

// 'order' is a number of 254 bits, such as 1n<<253n. Therefore, we initialize the mask as 1<<253
for (let mask = 1n << 253n; mask > 0; mask >>= 1n) {
if (e & mask) {
R0 = addPoint(R0, R1)
R1 = addPoint(R1, R1)
} else {
R1 = addPoint(R0, R1)
R0 = addPoint(R0, R0)
}

exp = addPoint(exp, exp)
rem = scalar.shiftRight(rem, BigInt(1))
}

return res
return R0
}

/**
Expand Down
61 changes: 60 additions & 1 deletion packages/baby-jubjub/tests/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,72 @@
import { babyjub } from "circomlibjs"
import { utils } from "ffjavascript"
import { Base8, Point, addPoint, inCurve, mulPointEscalar, packPoint, r, unpackPoint } from "../src"
import * as scalar from "@zk-kit/utils/scalar"
import { Base8, Point, addPoint, inCurve, mulPointEscalar, packPoint, r, unpackPoint, order, subOrder } from "../src"
import { tonelliShanks } from "../src/sqrt"

describe("BabyJubjub", () => {
const secretScalar = BigInt(324)
const id: Point<bigint> = [BigInt(0), BigInt(1)]

let publicKey: Point<bigint>

it("Test point addition and inCurve", async () => {
expect(inCurve(id)).toBeTruthy()
expect(inCurve([BigInt(1), BigInt(0)])).toBeFalsy()
expect(addPoint(id, id).toString()).toBe(id.toString())

const p1: Point<bigint> = [
BigInt("17777552123799933955779906779655732241715742912184938656739573121738514868268"),
BigInt("2626589144620713026669568689430873010625803728049924121243784502389097019475")
]
const p2: Point<bigint> = [
BigInt("16540640123574156134436876038791482806971768689494387082833631921987005038935"),
BigInt("20819045374670962167435360035096875258406992893633759881276124905556507972311")
]
const p3: Point<bigint> = [
BigInt("7916061937171219682591368294088513039687205273691143098332585753343424131937"),
BigInt("14035240266687799601661095864649209771790948434046947201833777492504781204499")
]

expect(inCurve(p1)).toBeTruthy()
expect(inCurve(p2)).toBeTruthy()
expect(inCurve(p3)).toBeTruthy()

expect(addPoint(p1, p2).toString()).toBe(p3.toString())
})

it("Test point multiplication with small values", async () => {
const P: Point<bigint> = [
BigInt("17777552123799933955779906779655732241715742912184938656739573121738514868268"),
BigInt("2626589144620713026669568689430873010625803728049924121243784502389097019475")
]

expect(mulPointEscalar(P, BigInt(0)).toString()).toBe(id.toString())
expect(mulPointEscalar(P, BigInt(1)).toString()).toBe(P.toString())
expect(mulPointEscalar(P, BigInt(2)).toString()).toBe(addPoint(P, P).toString())
expect(mulPointEscalar(P, BigInt(3)).toString()).toBe(addPoint(addPoint(P, P), P).toString())

expect(mulPointEscalar(id, BigInt(1)).toString()).toBe(id.toString())
expect(mulPointEscalar(id, BigInt(14134324)).toString()).toBe(id.toString())
})

it("Test base point order", async () => {
expect(scalar.shiftRight(order, BigInt(3))).toBe(subOrder)
const G: Point<bigint> = [
BigInt("995203441582195749578291179787384436505546430278305826713579947235728471134"),
BigInt("5472060717959818805561601436314318772137091100104008585924551046643952123905")
]
const p1: Point<bigint> = mulPointEscalar(G, BigInt(8) * subOrder)
expect(p1.toString()).toBe(id.toString())
const p2 = mulPointEscalar(Base8, subOrder)
expect(p2.toString()).toBe(id.toString())

const random = BigInt("38275423985628165")
expect(mulPointEscalar(Base8, random).toString()).toBe(
mulPointEscalar(Base8, random + BigInt(543523) * subOrder).toString()
)
})

it("Should add 1 point to the curve", async () => {
const p1: Point<bigint> = [BigInt(0), BigInt(1)]

Expand Down
Loading