o1-labs / o1js

TypeScript framework for zk-SNARKs and zkApps
https://docs.minaprotocol.com/en/zkapps/how-to-write-a-zkapp
Apache License 2.0
477 stars 105 forks source link

Hardcode the genesis timestamp because it is used as a constant in constraints #1588

Open dfstio opened 2 months ago

dfstio commented 2 months ago

this.network.timestamp gives incorrect time and throw "the permutation was not constructed correctly: final value" error when calling tx.prove() on devnet:

[4:09:35 PM] timestamp  : 1793613780000
[4:09:35 PM] currentTime: 1713359375661
[4:09:36 PM] timestamp  : 1793614140000
[4:09:36 PM] currentTime: 1713359376108
[4:09:36 PM] timestamp  : 1793614140000
[4:09:36 PM] currentTime: 1713359376108
Error when proving Timestamp.setValue()
 FAIL  experimental/timestamp.devnet.test.ts
  Devnet timestamp
    ✓ should deploy contract (167265 ms)
    ✕ should send tx using timestamp (1773 ms)

  ● Devnet timestamp › should send tx using timestamp

    the permutation was not constructed correctly: final value

      at Object.<anonymous>.module.exports.__wbindgen_error_new (o1js/dist/node/bindings/compiled/_node_bindings/plonk_wasm.cjs:9566:17)
      at <wasm_bindgen::JsError as core::convert::From<E>>::from::h2f475aac24e93697 (wasm:/wasm/01326956:1:4417337)
      at caml_pasta_fp_plonk_proof_create (wasm:/wasm/01326956:1:2842888)
      at Object.<anonymous>.module.exports.caml_pasta_fp_plonk_proof_create (o1js/dist/node/bindings/compiled/_node_bindings/plonk_wasm.cjs:1791:14)
      at caml_pasta_fp_plonk_proof_create (src/mina/src/lib/crypto/kimchi_bindings/js/bindings.js:789:15)
      at ../../../../../../workspace_root/src/mina/src/lib/crypto/kimchi_backend/pasta/vesta_based_plonk.ml:120:15

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 passed, 2 total

The same code works correctly on the local blockchain.

Test used:

import { describe, expect, it } from "@jest/globals";
import {
  Cache,
  AccountUpdate,
  Mina,
  fetchAccount,
  Field,
  SmartContract,
  method,
  state,
  State,
  UInt64,
  Provable,
  PrivateKey,
} from "o1js";
import { initBlockchain } from "../utils/testhelpers";

class Timestamp extends SmartContract {
  @state(Field) value = State<Field>();

  @method async setValue(value: Field, currentTime: UInt64) {
    const timestamp = this.network.timestamp.getAndRequireEquals();
    Provable.log("timestamp  :", timestamp);
    Provable.log("currentTime:", currentTime);
    this.value.set(value);
  }
}

describe("Devnet timestamp", () => {
  const app = PrivateKey.randomKeypair();
  const zkApp = new Timestamp(app.publicKey);
  let deployer: PrivateKey | undefined;

  it(`should deploy contract`, async () => {
    deployer = (await initBlockchain("devnet"))?.deployer;
    expect(deployer).toBeDefined();
    if (deployer === undefined) throw new Error("deployer is undefined");
    const cache: Cache = Cache.FileSystem("./cache");
    console.log("Compiling ...");
    await Timestamp.compile({ cache });
    console.log("Deploying...");

    const sender = deployer.toPublicKey();

    await fetchAccount({ publicKey: sender });
    const tx = await Mina.transaction(
      { sender, fee: "100000000", memo: "deploy" },
      async () => {
        AccountUpdate.fundNewAccount(sender);
        await zkApp.deploy({});
      }
    );
    const txSent = await tx.sign([deployer, app.privateKey]).send();
    console.log({
      status: txSent.status,
      hash: txSent.hash,
      errors: txSent.errors,
    });
    const txIncluded = await txSent.wait();
    console.log({ status: txIncluded.status, hash: txIncluded.hash });
  });

  it(`should send tx using timestamp`, async () => {
    expect(deployer).toBeDefined();
    if (deployer === undefined) throw new Error("deployer is undefined");
    const sender = deployer.toPublicKey();

    await fetchAccount({ publicKey: sender });
    await fetchAccount({ publicKey: app.publicKey });
    const tx = await Mina.transaction(
      { sender, fee: "100000000", memo: "setValue" },
      async () => {
        await zkApp.setValue(Field(100), UInt64.from(Date.now()));
      }
    );
    await tx.prove();
    const txSent = await tx.sign([deployer]).send();
    console.log({
      status: txSent.status,
      hash: txSent.hash,
      errors: txSent.errors,
    });
    const txIncluded = await txSent.wait();
    console.log({ status: txIncluded.status, hash: txIncluded.hash });
  });
});
dfstio commented 2 months ago

It can be related to https://github.com/MinaProtocol/mina/pull/15503

phn210 commented 1 month ago

Will there be any fix for this soon?

dfstio commented 1 month ago

Running this test produces every time different verification key:

import { describe, expect, it } from "@jest/globals";
import { Field, SmartContract, method, state, State, Mina } from "o1js";

export class TestContract extends SmartContract {
  @state(Field) value = State<Field>();

  @method async setValue(value: Field) {
    const timestamp = this.network.timestamp.getAndRequireEquals();
    this.value.set(value);
  }
}

describe("Compile v1", () => {
  it(`should compile`, async () => {
    const Local = await Mina.LocalBlockchain();
    Mina.setActiveInstance(Local);
    const vk = (await TestContract.compile()).verificationKey;
    console.log("vk", vk.hash.toJSON());
  });
});

Commenting the line

const timestamp = this.network.timestamp.getAndRequireEquals();

resolves this issue. In other, more complicated contracts, the verification key depends on the line where compile() is called. Moving the compile() up or down several lines changes the verification key, with the verification key being the function of the line where compile() is called - it is the same if compile() stays on the same line. Removing references to network.timestamp also resolves it.

mitschabaude commented 1 month ago

Running this test produces every time different verification key:

thanks, that's what I expected from the error message. there must be a bug in the implementation that uses a different constant every time the circuit is compiled

mitschabaude commented 3 weeks ago

After looking at the code, the problem is obvious: The genesisTimestamp returned from Mina.getNetworkConstants() is different depending on whether:

However, genesisTimestamp is used as a constant in constraints when using this.network.timestamp. So e.g., if you compile, then set the Mina instance to local blockchain, and then try to prove, you will hit a prover error (because the prover is not using the same circuit that the vk was compiled from).

The only possible fix is to hardcode a single genesisTimestamp (i.e., the one consistent with Mina mainnet) as a global constant, and use that for timestamp. It's probably best to not get that constant from networkConstants() because that name implies that it depends on the network, which it must not when used as a circuit constant.

dfstio commented 1 week ago

@mitschabaude Can you please, when fixing it, also take into consideration that: 1) As I understand, I need to call fetchLastBlock() to get the correct timestamp. fetchLastBlock() works on devnet and mainnet but gives Unknown Error: {} on the local blockchain 2) The Timestamps I see on devnet are almost two years ahead of the current time. As I understand, they are calculated from the devnet hardfork timestamp, not from the devnet genesis timestamp.

mitschabaude commented 1 week ago

As I understand, I need to call fetchLastBlock() to get the correct timestamp.

I don't follow, why wouldn't you just get the current timestamp with Date.now() or something?

dfstio commented 1 week ago

I don't follow, why wouldn't you just get the current timestamp with Date.now() or something?

As I understand, if I want to compare timestamp with some time in provable code, the provable code derives the timestamp from the genesis timestamp and globalSlotSinceGenesis, not from Date.now(), and to get the correct globalSlotSinceGenesis, I need to call await fetchLastBlock(). Is it correct?

mitschabaude commented 1 week ago

I don't follow, why wouldn't you just get the current timestamp with Date.now() or something?

As I understand, if I want to compare timestamp with some time in provable code, the provable code derives the timestamp from the genesis timestamp and globalSlotSinceGenesis, not from Date.now(), and to get the correct globalSlotSinceGenesis, I need to call await fetchLastBlock(). Is it correct?

You're right it gets the global slot! It doesn't call fetchLastBlock() for local blockchain though, instead it calls Mina.getNetworkState() of which local blockchain has its own implementation