centrifuge / centrifuge-chain

Centrifuge Chain: the layer-1 blockchain for real-world assets, built on Substrate.
https://centrifuge.io
GNU Lesser General Public License v3.0
182 stars 78 forks source link

migration: Add LP Gateway migration for outbound messages #1946

Closed cdamian closed 1 month ago

cdamian commented 1 month ago

Description

Add a migration that removes the LP gateway storages related to outbound messages.

Checklist:

cdamian commented 1 month ago

Testing this locally using:

try-runtime --runtime existing create-snapshot --uri [wss://fullnode.centrifuge.io:443](wss://fullnode.centrifuge.io/) cfg.snap

try-runtime --runtime target/release/wbuild/centrifuge-runtime/centrifuge_runtime.wasm on-runtime-upgrade --disable-spec-version-check snap -p cfg.snap

However, it errors out:

[2024-08-05T09:30:44Z ERROR runtime] panicked at /Users/cdamian/codebase/centrifuge-chain/runtime/common/src/migrations/liquidity_pools_gateway.rs:97:9:
    assertion `left == right` failed: OutboundMessageNonce should be empty!
      left: 55
     right: 0
thread 'main' panicked at cli/main.rs:324:10:
called `Result::unwrap()` on an `Err` value: Input("failed to execute TryRuntime_on_runtime_upgrade: Execution aborted due to trap: wasm trap: wasm `unreachable` instruction executed\nWASM backtrace:\nerror while executing at wasm backtrace:\n ...
lemunozm commented 1 month ago

That's normal. you have a ValueQuery in that storage. So if no entry is found, a default is returned. But that does not add any real entry in the storage. I think you should check that the value is 0.

wischli commented 1 month ago

The try-runtime CLI works now! It failed for Altair because in the pre_upgrade we are assuming the OutboundMessageNonceStore to be greater than 0 which obviously isn't the case for Altair. Normally, I would fix that but given that Altair is low priority, especially currently with the strict audit deadline, and is not expected to utilize LPs, we can ignore it IMO.