onomyprotocol / arc

Arc moves assets cross-chain to and from integrated blockchains.
Apache License 2.0
3 stars 10 forks source link

Other relaying considerations #105

Closed AaronKutch closed 1 year ago

AaronKutch commented 1 year ago

In PR #104 the most important finalization change is check_for_events in ethereum_event_watcher.rs, but I searched through the code and found get_last_checked_block and find_latest_valset that also relay information from the ethereum side. Before any of my changes, the latter two were coded to just use the latest changes, and I am suspicious of them. get_last_checked_block is concerning because it is used twice in a nested pattern in eth_oracle_main_loop, and I don't know if the possible implications of a invalidation from a reorg have been analyzed. find_latest_valset is also in the same situation, what would happen if a valset update was invalidated? For now I have changed them to used finalized blocks, and the test on ava-bridge doesn't show any problems I haven't already encountered from before the change. find_latest_valset delays results by no more than 20 minutes on the different chains, so I think I haven't introduced significant problems there.

AaronKutch commented 1 year ago

Ok, find_latest_valset is just for predetermining if a batch/logic call/valset update will be able to pass with the absolute latest valset. If there is some reorg on a valset update, it may cause temporary errors, but ultimately check_for_events will only return finalized events that must have occurred on the true finalized contract state. I think get_last_checked_block might be the same deal, I'm not sure about it yet.

AaronKutch commented 1 year ago

Ok, get_last_checked_block has no bearing on security since it is checking things that have already been relayed. Using finalized blocks might be an optimization, but if it misses it may be searching thousands of blocks (in the case of starting with a brand new contract) which we want to avoid. So, I will be only doing the finalization in check_for_events