Open aakoshh opened 11 months ago
@aakoshh my take would be using events is really killing two birds with one stone. Saving gas and avoids the look ahead/back completely.
I agree that https://github.com/consensus-shipyard/ipc-solidity-actors/pull/285 is a good alternative, I didn't know that top-down and bottom-up messages are handled completely separately when I made my comment about the "Alternative solution" in the ticket.
Issue type
Bug
Have you reproduced the bug with the latest dev version?
Yes
Version
v0.1.0
Custom code
Yes
OS platform and distribution
Linux
Describe the issue
See https://filecoinproject.slack.com/archives/C04JR5R1UL8/p1700126202426169
https://github.com/consensus-shipyard/fendermint/pull/433 introduced a mechanism where the
sync.rs
looks ahead for the next block hash to find cross messages at a certain height: https://github.com/consensus-shipyard/fendermint/blob/1cdeda3bcb946633235c11967bf279e09e6ed5b3/fendermint/vm/topdown/src/sync.rs#L410-L421This, however, is not reflected when a node is syncing with the chain and fetches synchronously: https://github.com/consensus-shipyard/fendermint/blob/1cdeda3bcb946633235c11967bf279e09e6ed5b3/fendermint/vm/interpreter/src/chain.rs#L265-L268
The crux of the issue is that if block
N
contains a top-down message, it will be stored in the contract at heightN
, but it will be only available for querying at blockN+1
because of deferred execution. The syncer is cheating by storing the effects at heightN
by trying to figure out the hash of (a non-finalized) blockN+1
(or the next non-null block in general) and executing a query there.By contrast the other version is querying block
N
looking for the effects at heightN
which aren't available, by definition.One relatively simple solution is to flip the problem. Instead of going this way:
we would go:
That is, instead of looking ahead of the finalized block
N
looking for a future block where it's effects appear, we would accept that we won't know the effects ofN
, but instead we would look back to find the last non-null block beforeN
, and send a query at blockN
asking about this previous height. This should be deterministic, in the past, available for lookup for anyone already finalizingN
, and also increase wheneverN
increases.Alternative solution
The other approach we discussed is instead of string data in the contract (which costs gas and is expensive) we would emit events for cross messages, which are stored in the SQLite database instead of the EVM store. However we also have to think about how we're going to do bottom-up checkpoints, where we currently look up these messages from the ledger and we know that reaching out to CometBFT is fraught with errors during startup, for example https://github.com/consensus-shipyard/fendermint/pull/426
Repro steps
At the moment we can reproduce locally by launching a node like this:
Relevant log output