fix: potential stuck fetches (#5631)

* test: invalid fetches should not block valid fetches * fix: only decrement counter if we can actually fetch
chainflip-io · Feb 12, 2025 · a5ff468 · a5ff468
1 parent 93d0603
commit a5ff468
Show file tree

Hide file tree

Showing 17 changed files with 155 additions and 71 deletions.
diff --git 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
diff --git a/api/bin/chainflip-broker-api/Cargo.toml b/api/bin/chainflip-broker-api/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 authors = ["Chainflip team <https://github.com/chainflip-io>"]
 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
@@ -3,7 +3,7 @@ authors = ["Chainflip team <https://github.com/chainflip-io>"]
 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
@@ -1,7 +1,7 @@
 [package]
 authors = ["Chainflip team <https://github.com/chainflip-io>"]
 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
@@ -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
@@ -3,11 +3,11 @@ authors = ["Chainflip team <https://github.com/chainflip-io>"]
 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
@@ -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
@@ -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 <https://github.com/chainflip-io>"]
 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
@@ -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
@@ -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
@@ -3,7 +3,7 @@ authors = ["Chainflip team <https://github.com/chainflip-io>"]
 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
@@ -8,7 +8,7 @@ license = "<TODO>"
 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
@@ -1634,32 +1634,52 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 									deposit_address,
 									deposit_fetch_id,
 									..
-								} =>
-									Self::should_fetch_or_transfer(
-										&mut maybe_no_of_fetches_remaining,
-									) && DepositChannelLookup::<T, I>::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::<T, I>::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,
 								),