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
509 stars 115 forks source link

Failed to derive correct actions issue #1775

Closed aii23 closed 1 month ago

aii23 commented 2 months ago

o1js version: 1.4.0

We have a problem getting right action. It always fails with "Failed to derive correct actions" error. Actions state hash that is generated locally do not match to the one, that is on contract. While when we trying to get same actions from GraphQL, we gets the right actions, and final state is equal to one on contract.

We have witness two strange behaviors (in gist files I marked inconsistency with !!!!) 1) When fetching actions with Mina.getActions and manually with GraphQL request it gets same actions, but hashes for them are different https://gist.github.com/aii23/ed548849d8f144648fffef0cb303aa6e 2) When fetching actions with Mina.getActions it can return different order of actions for the same account. https://gist.github.com/aii23/15528228d57ae99e2c099dd278cca33a

aii23 commented 2 months ago

We got this on devent. Contract B62qk3U9ZUZ2a21T4thqhUgBvXM3xMyA1HtQBQ79unUHrytEVpNhtqm. Error message without actions: Failed to derive correct actions hash for B62qk3U9ZUZ2a21T4thqhUgBvXM3xMyA1HtQBQ79unUHrytEVpNhtqm. Derived hash: 1435907438836098282690590072715440295298021373451541716662075799983484743534, expected hash: 806534191840875465837323299020552650224268402125665996458629125778783471442). ... Please try a different Archive Node API endpoint.

Actions provided separately in gist files

asimaranov commented 2 months ago

Created POC for the issue

image

Script: https://github.com/ZkNoid/L1-lottery/blob/main/scripts/bug_poc.ts

To reproduce:

git clone https://github.com/ZkNoid/L1-lottery
npm install
npm run build && node build/scripts/bug_poc.js
asimaranov commented 2 months ago

Also it's needed to create src/private_constants.js file with the following content:

import { PrivateKey } from "o1js";
export const { publicKey: treasury, privateKey: treasuryKey } = PrivateKey.randomKeypair();
asimaranov commented 2 months ago

Pinpointed the issue. There is one strange action that broke hash reconstruction. All actions before and after this action are fetched correctly. The strange action state is 806534191840875465837323299020552650224268402125665996458629125778783471442

So here is the simplified PoC of the issue that fails on fetching the strange event:

import { Field, Mina, PublicKey } from 'o1js';

const Network = Mina.Network({
  mina: 'https://api.minascan.io/node/devnet/v1/graphql',
  archive: 'https://api.minascan.io/archive/devnet/v1/graphql',
});

Mina.setActiveInstance(Network);

// Failed to derive correct actions hash
let result = await Mina.activeInstance.fetchActions(
  PublicKey.fromBase58(
    'B62qk3U9ZUZ2a21T4thqhUgBvXM3xMyA1HtQBQ79unUHrytEVpNhtqm'
  ),
  {
    fromActionState: Field.from(
      '23282384716759778041265197000989706423962846376707787360646691452073711502533'
    ),
    endActionState: Field.from(
      '806534191840875465837323299020552650224268402125665996458629125778783471442'
    ),
  }
);

console.log('Result', result);
asimaranov commented 2 months ago

The same reproduced on the new contract instance: B62qjH2N7qJVyMxrZSJdqfdnp97ZduUdydehjigpDN51k3cFNe8G2Dk

asimaranov commented 2 months ago

Pin-pointed the issue even more. The problem occurs when 3+ actions are dispatched within one block. Two works fine, but when there are 3+ transactions to our zkApp it fails.

Here are the transactions to our old zkApp in the block with strange action image

Transactions to the new zkApp in the block with strange action image

So when it becomes 3+, all fails

asimaranov commented 2 months ago

Reproduced this on lightnet. The combination of using this.sender.getAndRequireSignature() passed to an action and 3+ actions sending in one block are the required components for issue reproduction

We made a repository for error reproduction. It can be reproduced on lightnet https://github.com/aii23/actions_issue_poc

asimaranov commented 2 months ago

Issue is investigated and a possible fix is provided: https://github.com/o1-labs/o1js/pull/1784

45930 commented 2 months ago

Continuing the conversation from PR #1784 here in the issue.

I was able to get the reverse-reducer to fail in a case with the following graphQL response:

"actions": [
      {
        "actionData": [
          {
            "accountUpdateId": "17",
            "data": [
              "740971403016468965787475457550014020057780757554240833311239560123179876751",
              "0"
            ]
          },
          {
            "accountUpdateId": "18",
            "data": [
              "6655568289342731678656125453034105075199679752638202562827479720162057088854",
              "1"
            ]
          },
          {
            "accountUpdateId": "19",
            "data": [
              "2439184263142160415641297666865197782135086936853230980233563316847699451441",
              "1"
            ]
          },
          {
            "accountUpdateId": "19",
            "data": [
              "21477189566778948764678522962711803349256738445207596937073383562233772290060",
              "0"
            ]
          },
          {
            "accountUpdateId": "20",
            "data": [
              "21477189566778948764678522962711803349256738445207596937073383562233772290060",
              "0"
            ]
          },
          {
            "accountUpdateId": "21",
            "data": [
              "740971403016468965787475457550014020057780757554240833311239560123179876751",
              "0"
            ]
          },
          {
            "accountUpdateId": "21",
            "data": [
              "6655568289342731678656125453034105075199679752638202562827479720162057088854",
              "1"
            ]
          }
        ]
      }
    ]

This action data was created by dispatching multiple events from within the same transaction. They seem to be ordered correctly, but when multiple transactions dispatch events within the same block, they are out of order. My current approach is to try sorting the incoming events by their account update id, which appears to be a valid sorting key, but I don't know if that's actually in line with the protocol rules.

@MartinMinkov , do you know if accountUpdateId is ok to use as a sort key for ordering actions from the archive node?

MartinMinkov commented 2 months ago

@MartinMinkov , do you know if accountUpdateId is ok to use as a sort key for ordering actions from the archive node?

To be honest, I'm not sure. I think the aim was to sort by account update id, with this code here, so it's very possible there is a bug in the ordering code.

Without talking to someone who has more experience with the data flow of the daemon to the archive node, I think the best way is to verify through experiment and just try different combinations of actions and see if it passes the transaction logic.

asimaranov commented 2 months ago

Without talking to someone who has more experience

Could you please suggest someone to ping about the issue? It's a blocker for our ZkApp devnet launch. Would be amazing if this could be checked by an archive node developer for the faster issue solving

45930 commented 2 months ago

I pushed a new commit to the PR which does the sorting I propose 🤞

asimaranov commented 2 months ago

I pushed a new commit to the PR which does the sorting I propose 🤞

Our backend works fine with this fix

45930 commented 1 month ago

Hey there #1784 is merged. It should make it into the next release of o1js!