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

Odd behavior with nargo test (stack overflow) and nargo prove (same test pass but only with hexadecimal encoding) #2456

Closed
jat9292 opened this issue Aug 27, 2023 · 2 comments
Assignees
Labels
bug Something isn't working ssa

Comments

@jat9292
Copy link

jat9292 commented Aug 27, 2023

Aim

When working on a Private token project, a test that should pass panicked when I used nargo test with a "stack overflow" error. I then tried the same test via nargo prove by putting the same values for the input of main function in the Prover.toml file. Then unexpectedly again, with nargo prove I received this error Error: Expected witness values to be integers, provided value causes number too large to fit in target type error. This was not making a lot of sense, so I decided to convert my 3 big Field inputs from their decimal form to their hexadecimal form inside Prover.toml file. Then finally, nargo prove passed as expected from the beginning.

I am not sure if those two issues are related, but I create a unique, as both happened almost at the same time.

Expected Behavior

nargo test should have passed correctly, and nargo prove should have passed also with decimal values.

Bug

nargo test-> fatal runtime error: stack overflow
nargo prove(before hexadecimal conversion) -> Error: Expected witness values to be integers, provided value causes "number too large to fit in target type" error

To Reproduce

  1. Redo same steps as in first paragraph using the circuit from this branch : https://github.com/jat9292/Private-token/blob/a818f608cf22912183a49dc4e563affda5a985f0/circuits/transfer/src/main.nr

Installation Method

Binary

Nargo Version

nargo 0.10.3 (git version hash: 2db759f, is dirty: false)

Additional Context

No response

Would you like to submit a PR for this Issue?

No

Support Needs

No response

@kevaundray
Copy link
Contributor

@TomAFrench what is the status of this issue?

@kevaundray kevaundray added this to the 0.25 milestone Jan 15, 2024
@TomAFrench
Copy link
Member

TomAFrench commented Jan 16, 2024

The first issue is inherent to toml as it cannot hold a numeric value which wouldn't fit inside an i64.

I can reproduce the stack overflow issue, as part of this I've simplified the circuit to the below.

fn main(
    private_key: Field,
    randomness1: Field,
    randomness2: Field,
    value: u40,
    balance_old_me_clear: u40, // balance_old_me_clear is the clear (i.e decrypted) balance of sender, this is computed offchain by solving the DLP with babygiant algorithm, after calling bjj_exp_elgamal_decrypt with his private key
    public_key_me: pub Gaffine,
    public_key_to: pub Gaffine,
    balance_old_me_encrypted_1: pub Gaffine,
    balance_old_me_encrypted_2: Gaffine,
    balance_old_to_encrypted_1: pub Gaffine,
    balance_old_to_encrypted_2: pub Gaffine,
    balance_new_me_encrypted_1: pub Gaffine,
    balance_new_me_encrypted_2: pub Gaffine,
    balance_new_to_encrypted_1: pub Gaffine,
    balance_new_to_encrypted_2: pub Gaffine
) {
    let computed_public_key = bjj_priv_to_pub_key(private_key);
    assert_eq(public_key_me, computed_public_key); // check that the sender really owns the private key corresponding to his public key
    assert(value <= balance_old_me_clear); // the sender must have sufficient balance
    assert(value >= 1); // this is to deter potential front-running issue: ref https://crypto.stanford.edu/~buenz/papers/zether.pdf §3.1. Here we adopt a simpler approach than the multistep approach proposed in the Zether paper, for a better UX: an attacker who tries to DOS the sender should at least pay 1 token to either original sender or receiver. The "1" threshold could be changed to ensure correct economic incentives, typically this should be at least a multiple of the average gas price of a transfer transaction.
    // another more straightforward solution to front-running would be simply to do the homomorphic addition in the smart contract rather than the circuit, but this is too expensive today on Ethereum, according to the Zeestar paper §III : https://files.sri.inf.ethz.ch/website/papers/sp22-zeestar.pdf

    let bjj_affine: AffineCurve = baby_jubjub().curve;
    let base_pt: Gaffine = baby_jubjub().base8;
    let embedded_balance_old_me_clear = bjj_affine.mul(balance_old_me_clear as Field, base_pt);
    let decoded_value = bjj_exp_elgamal_decrypt(
        private_key,
        (balance_old_me_encrypted_1, balance_old_me_encrypted_2)
    );
    assert_eq(decoded_value, embedded_balance_old_me_clear); // check that unencrypted balance of sender really corresponds to his encrypted balance

    let balance_new_me_encrypted_computed = bjj_exp_elgamal_encrypt(public_key_me, balance_old_me_clear - value, randomness1);
    // checks that the new encrypted balance of sender is correct
    assert_eq(balance_new_me_encrypted_computed.1, balance_new_me_encrypted_2);
    assert_eq(balance_new_me_encrypted_computed.0, balance_new_me_encrypted_1);

    let value_encrypted_to = bjj_exp_elgamal_encrypt(public_key_to, value, randomness2);
    let balance_new_to_encrypted_computed = (
        bjj_affine.add(balance_old_to_encrypted_1, value_encrypted_to.0), bjj_affine.add(balance_old_to_encrypted_2, value_encrypted_to.1)
    ); // addition of the points on Baby Jubjub : this operation is additevely homomorphic for Exponential ElGamal
    // checks that the new encrypted balance of receiver is correct
    assert_eq(balance_new_to_encrypted_computed.0, balance_new_to_encrypted_1);
    assert_eq(balance_new_to_encrypted_computed.1, balance_new_to_encrypted_2);
}

When compiling this circuit I get a huge number of repeats blocks in SSA which just immediately jump to a different block. We should probably catch these earlier and do some minor inlining in this case.

  b32329():
    jmp b32330(Field 5299619240641551281634865583518297030282874472190772894086521144482721001553, Field -4938092073378617504287780177435440538246701238791326556475388250393169527414, Field -2749661043126392628337698819795691390981159337115536281098937116954595247277, Field 1)
  b32323():
    jmp b32324(Field 5299619240641551281634865583518297030282874472190772894086521144482721001553, Field -4938092073378617504287780177435440538246701238791326556475388250393169527414, Field -2749661043126392628337698819795691390981159337115536281098937116954595247277, Field 1)
  b32317():
    jmp b32318(Field 5299619240641551281634865583518297030282874472190772894086521144482721001553, Field -4938092073378617504287780177435440538246701238791326556475388250393169527414, Field -2749661043126392628337698819795691390981159337115536281098937116954595247277, Field 1)
  b32311():
    jmp b32312(Field 5299619240641551281634865583518297030282874472190772894086521144482721001553, Field -4938092073378617504287780177435440538246701238791326556475388250393169527414, Field -2749661043126392628337698819795691390981159337115536281098937116954595247277, Field 1)

@Savio-Sou Savio-Sou removed this from the 0.25 milestone Feb 24, 2024
@TomAFrench TomAFrench closed this as not planned Won't fix, can't repro, duplicate, stale Apr 21, 2024
@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Noir Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ssa
Projects
Archived in project
Development

No branches or pull requests

5 participants