search

consensus-shipyard / ipc

🌳 Spawn multi-level trees of customized, scalable, EVM-compatible networks with IPC. L2++ powered by FVM, Wasm, libp2p, IPFS/IPLD, and CometBFT.
https://ipc.space
Apache License 2.0
44 stars 39 forks source link

L2+ testing todos #1223

Open cryptoAtwill opened 2 days ago

cryptoAtwill commented 2 days ago

After an initial round of testing with L2+ feature, here is a list of todos to consider:

  1. Adding a doc on who, how is invoking cross messages calls. Because there were some assumption/requirements not properly documented yet, all the components needed to ensure correct functioning.
  2. Currently, there is missing a debug tool for cross message calls. It would be really helpful to have a tool, maybe external, that tracks the lifecycle of the cross network messages, i.e. where the message submitted, which subnet is the message now, is the message committed into the postbox and etc... This would help debug cross network message execution.
  3. Checking subnet.circSupply. Not sure if this variable is tracked correctly. This value is deducted by IPCEnvelop.value regardless message type? But only increased (after bootstrap) when message is Transfer.
  4. There should be a debug tool to check if the message execution is successful or failed. If failed, should be possible to show the error.
  5. Consistent way to encode ret bytes. The way that encodes the error is using encodeWithSelector, but for L2+ error is just encode, it's kind of confusing for debugging to tell which decode method to use.
  6. Emit event in test should be removed. Events should be emitted in actual code, not from test, otherwise the expect becomes useless.
karlem commented 2 days ago
  1. 100% agree. I will work on it today.
  2. I think we could use https://thegraph.com/cs/ for this
  3. This looks like a bug. Will fix today.
cryptoAtwill commented 1 day ago

Because of https://github.com/consensus-shipyard/ipc/issues/1225, the current approach of scanning events emitted from postbox will not work. There will be missing events.

cryptoAtwill commented 1 day ago

The nonce handling is incorrect. Only the first xnet message will work, subsequent ones will fail.

The scenario is sending TWO cross network message from L1 to L3. This is the first ever L1 topdown message, so for the cross message itself, the nonce is 0.

On L2, topdown finality will have the message applied. It gets committed with appliedNonce increase by 1. Then the committed message in postbox is propagated. Internally it calls commitValidatedCrossMessage and appliedNonce is incremented again by 1.

The overall effect is one xnet message from L1 to L3 will increase the applied nonce by 2. This is observed when testing in calibration.

The second cross network message from L1 to L3 will have nonce == 1, the nonce will not match.

cryptoAtwill commented 1 day ago

Potential double nonce increment.

Similar to the above, the appliedNonce is incremented first here. But subsequently, it also call sendReceipt. Internally, it will increment the appliedNonce by 1 as well. This will lead to one message increasing the nonce by 2.