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

add: multiproof consistency test #424

Merged

Conversation

agnxsh
Copy link
Collaborator

@agnxsh agnxsh commented Jul 7, 2024

Adds a Verkle multiproof consistency test, a continuation to addressing issue #396, implements the test https://github.com/crate-crypto/go-ipa/blob/b1e8a79/multiproof_test.go#L63-L132 as per the Verkle trie spec

Skips ipa_multi_verify because of a potential bug in g2t, will be addressed and fixed in successive PRs

Copy link
Owner

@mratsim mratsim left a comment

Choose a reason for hiding this comment

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

Very nice, so this confirms that generating multiproof is bug free.

@mratsim mratsim merged commit f55f644 into mratsim:master Jul 10, 2024
12 checks passed
@agnxsh
Copy link
Collaborator Author

agnxsh commented Jul 10, 2024

Very nice, so this confirms that generating multiproof is bug free.

Absolutely 🙂

@mratsim
Copy link
Owner

mratsim commented Aug 16, 2024

Currently investigating the g2t issue. I found another bug in the multiproof generation so we need more complete coverage: 1f8c376

@mratsim
Copy link
Owner

mratsim commented Aug 16, 2024

To clarify, the PR added the following test

var multiproof: IpaMultiProof[8, EC_TwEdw_Aff[Fp[Banderwagon]], Fr[Banderwagon]]
CRS.ipa_multi_prove(
domain, prover_transcript,
multiproof, polys,
commitments, opening_challenges_in_domain)
var prover_challenge {.noInit.}: Fr[Banderwagon]
prover_transcript.squeezeChallenge("state", prover_challenge)
doAssert prover_challenge.toHex(littleEndian) == "0xeee8a80357ff74b766eba39db90797d022e8d6dee426ded71234241be504d519", "Issue with squeezing the prover challenge"

to confirm that the state of the transcript matches with go-ipa.

However getQuotientPolyInDomain was wrongly implemented and was missing a multiplication here 1f8c376

meaning g was wrong here:

# Compute combining polynomial
# g(X) = ∑rⁱ.(fᵢ(X)-yᵢ)/(X-zᵢ) = r⁰(f₀(X)-y₀)/(X-z₀) + r¹(f₁(X)-y₁)/(X-z₁) + ... + rⁿ⁻¹(fₙ₋₁(X)-yₙ₋₁)/(X-zₙ₋₁)
# As yᵢ = fᵢ(zᵢ)
# quotient_poly(rⁱfᵢ(X), zᵢ) == rⁱ.quotient_poly(fᵢ(X), zᵢ)
block:
domain.getQuotientPolyInDomain(
g[],
polys_by_challenges[0],
sparse_challenges[0])
let t = g2 # We use g2 as temporary
t.zeroMem(sizeof(t[]))
for i in 1 ..< num_distinct_challenges:
domain.getQuotientPolyInDomain(
t[],
polys_by_challenges[i],
sparse_challenges[i])
g[] += t[]
# Commit to the combining polynomial g(X) = ∑rⁱ.(fᵢ(X)-yᵢ)/(X-zᵢ)
var D {.noInit.}: EC
crs.pedersen_commit(D, g[])
proof.D.affine(D)
transcript.absorb("D", proof.D)

D is derived from committing to g(X) and then absorbed into the transcript. I'm not sure why the transcript was correct for that consistency test but it does outline that we need a more comprehensive test suite #396

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants