movementlabsxyz / movement

The Movement Network is a Move-based L2 on Ethereum.
Apache License 2.0
74 stars 61 forks source link

Persist Relayer Transfers #775

Open 0xmovses opened 1 week ago

0xmovses commented 1 week ago

When the relayer is restarted the pullstate file gets wiped. We need to:

musitdev commented 1 week ago

Why the pullstate.store feil is wiped? The relayer didn't remove it.

0xmovses commented 1 week ago

When the service is restarted with systemctl restart <service> the array

INFO bridge_service: Bridge current transfer processing [ ]

Is always empty, no pending transfers are persisted. It could be a fix we need to make within the docker compose files.

0xmovses commented 1 week ago

I'm doing a local test now to check its working as expected locally.

musitdev commented 1 week ago

Could we get them from the blockchain by requesting bridgeTransfers fields of the Initiator contract? I should be more reliable. We've to ensure that transfer finished are removed from the list. So by getting the list we get all pending transfer.

0xmovses commented 1 week ago

Yes, that's a good idea, the contract will return the state of each transfer and that should be our source of truth. I'll start working on this.

andygolay commented 6 days ago

Could we get them from the blockchain by requesting bridgeTransfers fields of the Initiator contract? I should be more reliable. We've to ensure that transfer finished are removed from the list. So by getting the list we get all pending transfer.

When I call bridgeTransfers() for example like

            InitiatorContract::Move(instance) => {
                for (bridge_transfer_id, transfer) in instance.bridgeTransfers()

It says it's expecting an arg. And the contract does seem like you're supposed to do bridgeTransfers(<bridgeTransferId>) to return details of one transfer.

See https://github.com/movementlabsxyz/movement/blob/main/protocol-units/bridge/contracts/src/AtomicBridgeInitiatorMOVE.sol#L26

So it seems like I'll need to add a view function to the contract, to get all the transfers... does that seem correct or am I missing some way to view all the transfers?

However to make a view function work, we'll need to store the bridge transfer IDs somewhere as well, because mapping doesn't appear to support length.

Maybe it's better to simply, on each run_bridge loop, cache state_runtime into some file that persists? That way we don't need to do any blockchain function calls, and we are guaranteed to keep the correct runtime state?

musitdev commented 6 days ago

It's better to rely on the blockchain than on a file. As transfer Id are managed onchain we should use the onchain solution. Perhaps it needs to update the smart contract, but it's better.

musitdev commented 6 days ago

What is hot transfer? I propose that indexer has only one source of truth, the blockchains we bridge. If you use the relayer, the relayer become the source of truth, that less true. Relayer can have issue, bug or crash more than the blockchain.