diff --git a/CHANGELOG.md b/CHANGELOG.md index cc0c83437f..ce69e9e00a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ All notable changes included in each Chainflip release will be documented in this file. +## [1.8.2] - 2025-02-12 + +- Prevent blocking of fetches due to small edge case ([#5631](https://github.com/chainflip-io/chainflip-backend/pull/5631)) + ## [1.8.1] - 2025-02-11 ### Features diff --git a/Cargo.lock b/Cargo.lock index b75678752c..6211f37ef0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1342,7 +1342,7 @@ dependencies = [ [[package]] name = "cf-engine-dylib" -version = "1.8.1" +version = "1.8.2" dependencies = [ "chainflip-engine", "engine-proc-macros", @@ -1582,7 +1582,7 @@ dependencies = [ [[package]] name = "chainflip-api" -version = "1.8.1" +version = "1.8.2" dependencies = [ "anyhow", "async-trait", @@ -1629,7 +1629,7 @@ dependencies = [ [[package]] name = "chainflip-broker-api" -version = "1.8.1" +version = "1.8.2" dependencies = [ "anyhow", "cf-chains", @@ -1653,7 +1653,7 @@ dependencies = [ [[package]] name = "chainflip-cli" -version = "1.8.1" +version = "1.8.2" dependencies = [ "anyhow", "bigdecimal", @@ -1674,7 +1674,7 @@ dependencies = [ [[package]] name = "chainflip-engine" -version = "1.8.1" +version = "1.8.2" dependencies = [ "anyhow", "async-broadcast", @@ -1814,7 +1814,7 @@ dependencies = [ [[package]] name = "chainflip-lp-api" -version = "1.8.1" +version = "1.8.2" dependencies = [ "anyhow", "cf-primitives", @@ -1841,7 +1841,7 @@ dependencies = [ [[package]] name = "chainflip-node" -version = "1.8.1" +version = "1.8.2" dependencies = [ "cf-chains", "cf-primitives", @@ -3268,7 +3268,7 @@ checksum = "c34f04666d835ff5d62e058c3995147c06f42fe86ff053337632bca83e42702d" [[package]] name = "engine-proc-macros" -version = "1.8.1" +version = "1.8.2" dependencies = [ "engine-upgrade-utils", "proc-macro2", @@ -3278,7 +3278,7 @@ dependencies = [ [[package]] name = "engine-runner" -version = "1.8.1" +version = "1.8.2" dependencies = [ "anyhow", "assert_cmd", @@ -13269,7 +13269,7 @@ checksum = "a8f112729512f8e442d81f95a8a7ddf2b7c6b8a1a6f509a95864142b30cab2d3" [[package]] name = "state-chain-runtime" -version = "1.8.1" +version = "1.8.2" dependencies = [ "bitvec", "cf-amm", diff --git a/api/bin/chainflip-broker-api/Cargo.toml b/api/bin/chainflip-broker-api/Cargo.toml index 6aea1ef3fa..53f397c921 100644 --- a/api/bin/chainflip-broker-api/Cargo.toml +++ b/api/bin/chainflip-broker-api/Cargo.toml @@ -1,7 +1,7 @@ [package] authors = ["Chainflip team "] name = "chainflip-broker-api" -version = "1.8.1" +version = "1.8.2" edition = "2021" [package.metadata.deb] diff --git a/api/bin/chainflip-cli/Cargo.toml b/api/bin/chainflip-cli/Cargo.toml index 8879e2d951..4eb0bc5905 100644 --- a/api/bin/chainflip-cli/Cargo.toml +++ b/api/bin/chainflip-cli/Cargo.toml @@ -3,7 +3,7 @@ authors = ["Chainflip team "] edition = "2021" build = "build.rs" name = "chainflip-cli" -version = "1.8.1" +version = "1.8.2" [lints] workspace = true diff --git a/api/bin/chainflip-lp-api/Cargo.toml b/api/bin/chainflip-lp-api/Cargo.toml index 843b133b59..da917bf3a7 100644 --- a/api/bin/chainflip-lp-api/Cargo.toml +++ b/api/bin/chainflip-lp-api/Cargo.toml @@ -1,7 +1,7 @@ [package] authors = ["Chainflip team "] name = "chainflip-lp-api" -version = "1.8.1" +version = "1.8.2" edition = "2021" [package.metadata.deb] diff --git a/api/lib/Cargo.toml b/api/lib/Cargo.toml index a5cfaf1490..6c48bfd7da 100644 --- a/api/lib/Cargo.toml +++ b/api/lib/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "chainflip-api" -version = "1.8.1" +version = "1.8.2" edition = "2021" [lints] diff --git a/engine-dylib/Cargo.toml b/engine-dylib/Cargo.toml index d96710996e..8490aa07fe 100644 --- a/engine-dylib/Cargo.toml +++ b/engine-dylib/Cargo.toml @@ -3,11 +3,11 @@ authors = ["Chainflip team "] build = "build.rs" edition = "2021" name = "cf-engine-dylib" -version = "1.8.1" +version = "1.8.2" [lib] crate-type = ["cdylib"] -name = "chainflip_engine_v1_8_1" +name = "chainflip_engine_v1_8_2" path = "src/lib.rs" [dependencies] diff --git a/engine-proc-macros/Cargo.toml b/engine-proc-macros/Cargo.toml index 1625a21035..e6e0eb1b5e 100644 --- a/engine-proc-macros/Cargo.toml +++ b/engine-proc-macros/Cargo.toml @@ -5,7 +5,7 @@ edition = "2021" name = "engine-proc-macros" # The version here is the version that will be used for the generated code, and therefore will be the # suffix of the generated engine entrypoint. TODO: Fix this. -version = "1.8.1" +version = "1.8.2" [lib] proc-macro = true diff --git a/engine-runner-bin/Cargo.toml b/engine-runner-bin/Cargo.toml index efe4215430..f22fb6ca5a 100644 --- a/engine-runner-bin/Cargo.toml +++ b/engine-runner-bin/Cargo.toml @@ -2,7 +2,7 @@ name = "engine-runner" description = "The central runner for the chainflip engine, it requires two shared library versions to run." # NB: When updating this version, you must update the debian assets appropriately too. -version = "1.8.1" +version = "1.8.2" authors = ["Chainflip team "] build = "build.rs" edition = "2021" @@ -22,10 +22,10 @@ assets = [ # to specify this. We do this in the `chainflip-engine.service` files, so the user does not need to set it # manually. [ - "target/release/libchainflip_engine_v1_8_1.so", + "target/release/libchainflip_engine_v1_8_2.so", # This is the path where the engine dylib is searched for on linux. # As set in the build.rs file. - "usr/lib/chainflip-engine/libchainflip_engine_v1_8_1.so", + "usr/lib/chainflip-engine/libchainflip_engine_v1_8_2.so", "755", ], # The old version gets put into target/release/deps by the package github actions workflow. diff --git a/engine-runner-bin/src/main.rs b/engine-runner-bin/src/main.rs index 4e58f4d377..8703b63ef1 100644 --- a/engine-runner-bin/src/main.rs +++ b/engine-runner-bin/src/main.rs @@ -13,7 +13,7 @@ mod old { } mod new { - #[engine_proc_macros::link_engine_library_version("1.8.1")] + #[engine_proc_macros::link_engine_library_version("1.8.2")] extern "C" { fn cfe_entrypoint( c_args: engine_upgrade_utils::CStrArray, diff --git a/engine-upgrade-utils/src/lib.rs b/engine-upgrade-utils/src/lib.rs index 47ab3dc077..53593e4046 100644 --- a/engine-upgrade-utils/src/lib.rs +++ b/engine-upgrade-utils/src/lib.rs @@ -11,7 +11,7 @@ pub mod build_helpers; // relevant crates. // Should also check that the compatibility function below `args_compatible_with_old` is correct. pub const OLD_VERSION: &str = "1.7.9"; -pub const NEW_VERSION: &str = "1.8.1"; +pub const NEW_VERSION: &str = "1.8.2"; pub const ENGINE_LIB_PREFIX: &str = "chainflip_engine_v"; pub const ENGINE_ENTRYPOINT_PREFIX: &str = "cfe_entrypoint_v"; diff --git a/engine/Cargo.toml b/engine/Cargo.toml index 10b4a7a24d..f6b241d68c 100644 --- a/engine/Cargo.toml +++ b/engine/Cargo.toml @@ -3,7 +3,7 @@ authors = ["Chainflip team "] build = "build.rs" edition = "2021" name = "chainflip-engine" -version = "1.8.1" +version = "1.8.2" [lib] crate-type = ["lib"] diff --git a/state-chain/node/Cargo.toml b/state-chain/node/Cargo.toml index 4fd605c91e..4ef20bdd0a 100644 --- a/state-chain/node/Cargo.toml +++ b/state-chain/node/Cargo.toml @@ -8,7 +8,7 @@ license = "" name = "chainflip-node" publish = false repository = "https://github.com/chainflip-io/chainflip-backend" -version = "1.8.1" +version = "1.8.2" [[bin]] name = "chainflip-node" diff --git a/state-chain/pallets/cf-ingress-egress/src/lib.rs b/state-chain/pallets/cf-ingress-egress/src/lib.rs index 903a2917d7..11625791c3 100644 --- a/state-chain/pallets/cf-ingress-egress/src/lib.rs +++ b/state-chain/pallets/cf-ingress-egress/src/lib.rs @@ -1634,32 +1634,52 @@ impl, I: 'static> Pallet { deposit_address, deposit_fetch_id, .. - } => - Self::should_fetch_or_transfer( - &mut maybe_no_of_fetches_remaining, - ) && DepositChannelLookup::::mutate( - deposit_address, - |details| { - details - .as_mut() - .map(|details| { - let can_fetch = - details.deposit_channel.state.can_fetch(); - - if can_fetch { - deposit_fetch_id.replace( - details.deposit_channel.fetch_id(), - ); - details + } => { + // Either: + // 1. We always want to fetch + // 2. We have a restriction on fetches, in which case we need to + // have fetches remaining. + // And we must be able to fetch the channel (it must exist and + // can_fetch must be true) + if (maybe_no_of_fetches_remaining.is_none_or(|n| n > 0)) && + DepositChannelLookup::::mutate( + deposit_address, + |details| { + details + .as_mut() + .map(|details| { + let can_fetch = details .deposit_channel .state - .on_fetch_scheduled(); - } - can_fetch - }) - .unwrap_or(false) - }, - ), + .can_fetch(); + + if can_fetch { + deposit_fetch_id.replace( + details.deposit_channel.fetch_id(), + ); + details + .deposit_channel + .state + .on_fetch_scheduled(); + } + can_fetch + }) + .unwrap_or(false) + }, + ) { + if let Some(n) = maybe_no_of_fetches_remaining.as_mut() { + *n = n.saturating_sub(1); + } + true + } else { + // If we have a restriction on fetches, but we have no fetch + // slots remaining then we don't want to fetch any + // more. OR: + // If the channel is expired / `can_fetch` returns + // false then we can't/shouldn't fetch. + false + } + }, FetchOrTransfer::Transfer { .. } => Self::should_fetch_or_transfer( &mut maybe_no_of_transfers_remaining, ), diff --git a/state-chain/pallets/cf-ingress-egress/src/tests.rs b/state-chain/pallets/cf-ingress-egress/src/tests.rs index b1beeb6c81..6fb84ef07c 100644 --- a/state-chain/pallets/cf-ingress-egress/src/tests.rs +++ b/state-chain/pallets/cf-ingress-egress/src/tests.rs @@ -1695,36 +1695,48 @@ fn do_not_batch_more_transfers_than_the_limit_allows() { }); } +fn trigger_n_fetches(n: usize) -> Vec { + let mut channel_addresses = vec![]; + + const ASSET: EthAsset = EthAsset::Eth; + + for i in 1..=n { + let (_, address, ..) = IngressEgress::request_liquidity_deposit_address( + i.try_into().unwrap(), + ASSET, + 0, + ForeignChainAddress::Eth(Default::default()), + ) + .unwrap(); + + let address: ::ChainAccount = address.try_into().unwrap(); + + channel_addresses.push(address); + + assert_ok!(IngressEgress::process_channel_deposit_full_witness_inner( + &DepositWitness { + deposit_address: address, + asset: ASSET, + amount: DEFAULT_DEPOSIT_AMOUNT, + deposit_details: Default::default(), + }, + Default::default() + )); + } + + channel_addresses +} + #[test] fn do_not_batch_more_fetches_than_the_limit_allows() { new_test_ext().execute_with(|| { MockFetchesTransfersLimitProvider::enable_limits(); const EXCESS_FETCHES: usize = 1; - const ASSET: EthAsset = EthAsset::Eth; let fetch_limits = MockFetchesTransfersLimitProvider::maybe_fetches_limit().unwrap(); - for i in 1..=fetch_limits + EXCESS_FETCHES { - let (_, address, ..) = IngressEgress::request_liquidity_deposit_address( - i.try_into().unwrap(), - ASSET, - 0, - ForeignChainAddress::Eth(Default::default()), - ) - .unwrap(); - let address: ::ChainAccount = address.try_into().unwrap(); - - assert_ok!(IngressEgress::process_channel_deposit_full_witness_inner( - &DepositWitness { - deposit_address: address, - asset: ASSET, - amount: DEFAULT_DEPOSIT_AMOUNT, - deposit_details: Default::default(), - }, - Default::default() - )); - } + trigger_n_fetches(fetch_limits + EXCESS_FETCHES); let scheduled_egresses = ScheduledEgressFetchOrTransfer::::get(); @@ -1738,6 +1750,7 @@ fn do_not_batch_more_fetches_than_the_limit_allows() { let scheduled_egresses = ScheduledEgressFetchOrTransfer::::get(); + // We should have fetched all except the exceess fetch. assert_eq!(scheduled_egresses.len(), EXCESS_FETCHES, "Wrong amount of left egresses!"); IngressEgress::on_finalize(2); @@ -1748,6 +1761,53 @@ fn do_not_batch_more_fetches_than_the_limit_allows() { }); } +#[test] +fn invalid_fetches_do_not_get_scheduled_and_do_not_block_other_fetches() { + new_test_ext().execute_with(|| { + MockFetchesTransfersLimitProvider::enable_limits(); + + const EXCESS_FETCHES: usize = 5; + + let fetch_limits = MockFetchesTransfersLimitProvider::maybe_fetches_limit().unwrap(); + + assert!( + fetch_limits > EXCESS_FETCHES, + "We assume excess_fetches can be processed in a single on_finalize for this test" + ); + + let channel_addresses = trigger_n_fetches(fetch_limits + EXCESS_FETCHES); + + assert_eq!( + ScheduledEgressFetchOrTransfer::::get().len(), + fetch_limits + EXCESS_FETCHES, + "All the fetches should have been scheduled!" + ); + + for address in channel_addresses.iter().take(fetch_limits) { + IngressEgress::recycle_channel(&mut Weight::zero(), *address); + } + + IngressEgress::on_finalize(1); + + // Check the addresses are the same as the expired ones, we can do this by comparing + // the scheduled egresses with the expired addresses + assert_eq!( + ScheduledEgressFetchOrTransfer::::get() + .iter() + .filter_map(|f_or_t| match f_or_t { + FetchOrTransfer::Fetch { deposit_address, .. } => Some(*deposit_address), + _ => None, + }) + .collect::>(), + channel_addresses[0..fetch_limits], + // Note: Ideally this shouldn't be the case since we don't want to keep holding fetches + // that will never be scheduled. However, at least we do not block ones that can be + // scheduled. + "The channels that expired should be the same as the scheduled egresses!" + ); + }); +} + #[test] fn do_not_process_more_ccm_swaps_than_allowed_by_limit() { new_test_ext().execute_with(|| { diff --git a/state-chain/runtime/Cargo.toml b/state-chain/runtime/Cargo.toml index 05f86e8371..88e69b3382 100644 --- a/state-chain/runtime/Cargo.toml +++ b/state-chain/runtime/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "state-chain-runtime" -version = "1.8.1" +version = "1.8.2" authors = ["Chainflip Team "] edition = "2021" homepage = "https://chainflip.io" diff --git a/state-chain/runtime/src/lib.rs b/state-chain/runtime/src/lib.rs index 825226506c..5f10989cbd 100644 --- a/state-chain/runtime/src/lib.rs +++ b/state-chain/runtime/src/lib.rs @@ -212,7 +212,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("chainflip-node"), impl_name: create_runtime_str!("chainflip-node"), authoring_version: 1, - spec_version: 181, + spec_version: 182, impl_version: 1, apis: RUNTIME_API_VERSIONS, transaction_version: 13,