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

SnarkyJS error in fetchEvents when multiple event types exist #627

Closed
JPGarzonE opened this issue Dec 4, 2022 · 0 comments · Fixed by #628
Closed

SnarkyJS error in fetchEvents when multiple event types exist #627

JPGarzonE opened this issue Dec 4, 2022 · 0 comments · Fixed by #628

Comments

@JPGarzonE
Copy link

JPGarzonE commented Dec 4, 2022

Overview

Our team Solus is building a Mixer for Mina Protocol. For the Mixer to work we are emitting a Deposit event as a primitive approach to store the Merkle Tree outside the smart contract, also we are emitting a Nullifier event to track withdrawals and avoid double-spending. It looks something like this:

export class MixerZkApp extends SmartContract {
	...
	events = {
		deposit: DepositClass,
    nullifier: NullifierClass
	}
	...
	@method updateMerkleTree(commitment: Field) {
		...
		this.emitEvent('deposit', deposit);
		...
	}
	@method emitNullifierEvent(nullifierHash: Field) {
		...
		this.emitEvent('nullifier', nullifierEvent);
		...
	}
}

Until here everything is ok. The problem comes when we fetch the events from the smart contract.

...
let zkapp = new MixerZkApp(zkappAddress);
let rawEvents = await zkapp.fetchEvents();

On the first request the events are fetched and all it’s fine. But, in the second request ( await zkapp.fetchEvents() ) this error starts to appear:

image

We needed this logic to work to finish the Mixer so we debugged to find if the mistake was on our side. After debugging for a while, we found that the error comes directly from SnarkyJS. Specifically, it happens in the fetchEvents function of the SmartContract class: https://github.com/o1-labs/snarkyjs/blob/main/src/lib/zkapp.ts#L971.

Conditions to replicate the error

  1. The smart contract has more than one type of event.
  2. The smartContract.fetchEvents(…) function is called more than once.
  3. The smart contract is executed on a local blockchain.

The error in SmartContract.fetchEvents

The problem that is happening between the first and the second request lies on the way Mina stores the events in the LocalBlockchain and the instruction: event.shift() in the line 1004 (https://github.com/o1-labs/snarkyjs/blob/main/src/lib/zkapp.ts#L1004).

In the Local Blockchain events are stored as an object in memory:

const events: Record<string, any> = {};

When the events of a specific Public Key are fetched from Mina (the Local Blockchain) the function fetchEvents(...) is not returning a new object, it is returning a reference to the events object that already exists. This is due to the way how JS stores compound objects in memory.

// mina.ts in the Local Mina Blockchain
// Line 510:
async fetchEvents(
  publicKey: PublicKey,
  tokenId: Field = TokenId.default
): Promise<any[]> {
  return events?.[publicKey.toBase58()]?.[TokenId.toBase58(tokenId)] ?? [];
}
// zkapp.ts in SmartContract.fetchEvents(..)
let events = (await Mina.fetchEvents(this.address, this.self.body.tokenId))
      .filter((el: any) => {
        let slot = UInt32.from(el.slot);
        return end === undefined
          ? start.lte(slot).toBoolean()
          : start.lte(slot).toBoolean() && slot.lte(end).toBoolean();
      })
      .map((el: any) => el.events)
      .flat();
// events = [event, event]
// Where each event inside events is a reference to the original event stored in memory.

Each one of the events come as an array in the following format:

events[i] = event = [eventTypeIndex, ...n]
// where n are the n parameters that the event has.

When the map function iterates over each event if the smart contract has more than one type of event it executes the following code:

...
} else {
  // if there are multiple events we have to use the index event[0] to find the exact event type
  let type = sortedEventTypes[event[0]];
  // all other elements of the array are values used to construct the original object, we can drop the first value since its just an index
  event.shift();
  return {
    type,
    event: this.events[type].fromFields(
      event.map((f: string) => Field(f))
    ),
  };
}

The problem happens in line 1007 (https://github.com/o1-labs/snarkyjs/blob/main/src/lib/zkapp.ts#L1007) when this.events[type] ends up being undefined and therefore this.events[type].fromFields fails. The reason why this.events[type] is undefined is because in the second request we make (zkapp.fetchEvents() )the variable type is undefined, while in the first request no error occurs because type is indeed the type of the event. In our case “deposit” or “nullifier”.

When event.shift() is executed it deletes the first element of the event array and is logical to think that it doesn’t modify anything beyond the local scope of the callback function inside events.map , because the map function by itself is non-destructive. However, the callback inside the map function could be destructive and each event inside map() is not a new array, but a reference pointing to the original event array that was fetched from the Mina local blockchain at the beginning of the function. So the original events object ends up being mutated.

That is why on the second request to smartcontract.fetchEvents(...) the type is undefined, this happens because the element at the first index of the event array is not anymore the index of the type of event (it was deleted in the last execution with the event.shift()). Instead it is the element that should be at event[1].

Quick solution

It could be solved using a non-destructive function such as .slice() that creates a new array from the original event array.

...
} else {
  // if there are multiple events we have to use the index event[0] to find the exact event type
  let type = sortedEventTypes[event[0]];
  // all other elements of the array are values used to construct the original object, we can drop the first value since its just an index
  let eventProps = event.slice(1);
  return {
    type,
    event: this.events[type].fromFields(
      eventProps.map((f: string) => Field(f))
    ),
  };
}

Finally

It is useful to clarify that this error is only an error that happens in local testing when the local blockchain is executed.

When the fetchEvents function is implemented in the original Mina network, a remote request to the blockchain is going to be implemented instead of a simple in-memory reference to an events object stored in local memory.

async fetchEvents() {
  throw Error(
    'fetchEvents() is not implemented yet for remote blockchains.'
  );
},

I hope the explanation was not too long.
Thank you for reading guys.

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 a pull request may close this issue.

1 participant