From d61103f06dd989b5326770b658890605e77acbbe Mon Sep 17 00:00:00 2001 From: Marcel Moura <5615598+marcelstanley@users.noreply.github.com> Date: Mon, 26 Aug 2024 15:04:37 -0300 Subject: [PATCH 1/2] fix(experimental): log duplicate addresses This fixes an issue where the redis set was being manipulated during the update process. --- CHANGELOG.md | 2 +- offchain/authority-claimer/src/listener.rs | 29 ---------------------- offchain/rollups-events/src/broker/mod.rs | 9 +++---- 3 files changed, 5 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a343c93f..16c10656f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). -## [1.5.1] 2024-08-16 +## [1.5.1] 2024-08-26 ### Added diff --git a/offchain/authority-claimer/src/listener.rs b/offchain/authority-claimer/src/listener.rs index 4ea45482e..9ab334d0f 100644 --- a/offchain/authority-claimer/src/listener.rs +++ b/offchain/authority-claimer/src/listener.rs @@ -531,35 +531,6 @@ mod tests { assert_listen(&mut listener, &dapps, &vec![0]).await; } - #[tokio::test] - async fn multidapp_listen_with_duplicate_dapps() { - let docker = Cli::default(); - let (fixture, mut listener, dapps) = - setup_multidapp_listener(&docker, false).await.unwrap(); - fixture.dapps_set(vec![]).await; - - // Initializes with 0 addresses in the set. - assert_eq!(0, fixture.dapps_members().await.len()); - - // We add a lowercase and an uppercase version of the same address. - let dapp: Address = [10; 20].into(); - fixture.dapps_add(dapp.to_string().to_lowercase()).await; - fixture.dapps_add(dapp.to_string().to_uppercase()).await; - - // We now have 2 addresses in the set. - assert_eq!(2, fixture.dapps_members().await.len()); - - // We then produce some claims and listen for them. - let mut epochs = vec![0; dapps.len()]; - let indexes = vec![2, 2, 0]; - multidapp_produce_claims(&fixture, &mut epochs, &dapps, &indexes).await; - let indexes = vec![2, 2]; - assert_listen(&mut listener, &dapps, &indexes).await; - - // Now we have 1 address because one of the duplicates got deleted. - assert_eq!(1, fixture.dapps_members().await.len()); - } - #[tokio::test] async fn multidapp_listen_with_changing_dapps() { let docker = Cli::default(); diff --git a/offchain/rollups-events/src/broker/mod.rs b/offchain/rollups-events/src/broker/mod.rs index 56e18ab51..1883f46fd 100644 --- a/offchain/rollups-events/src/broker/mod.rs +++ b/offchain/rollups-events/src/broker/mod.rs @@ -380,11 +380,10 @@ impl Broker { match dapp_address { Ok(dapp_address) => { if dapp_addresses.contains(&dapp_address) { - let _: () = self - .connection - .clone() - .srem(DAPPS_KEY, value) - .await?; + tracing::info!( + "Ignored duplicate DApp address {:?}", + value, + ) } else { dapp_addresses.push(dapp_address); } From 6abf9990f39e62323ed2ec9d727195d05969836d Mon Sep 17 00:00:00 2001 From: Marcel Moura <5615598+marcelstanley@users.noreply.github.com> Date: Mon, 26 Aug 2024 18:33:45 -0300 Subject: [PATCH 2/2] docs(experimental): update multidapp-claimer instructions --- offchain/authority-claimer/README.md | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/offchain/authority-claimer/README.md b/offchain/authority-claimer/README.md index 068689fac..665231cfa 100644 --- a/offchain/authority-claimer/README.md +++ b/offchain/authority-claimer/README.md @@ -17,13 +17,22 @@ All dapps must share the same History contract and the same chain ID. Instead of using evironment variables, the claimer will get the list of application addresses from Redis, through the `experimental-dapp-addresses-config` key. -This key holds a Redis Set value. -You must use commands such as SADD and SREM to manipulate the list of addresses. -Addresses are encoded as hex strings without the leading `"0x"`. -Redis values are case sensitive, so addresses must be in lowercase format. -Example address value: `"0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a"`. +This key holds a [Redis Set](https://redis.io/docs/latest/develop/data-types/sets/) value. +You must use commands such as [`SADD`](https://redis.io/docs/latest/commands/sadd/) and [`SREM`]https://redis.io/docs/latest/commands/srem/() to manipulate the set of addresses. -You may rewrite the list of addresses at any time, +Application addresses must be encoded as hex strings. +The prefix `0x` is ignored by the `authority-claimer` when loading applications adresses. +However, it's advised to use it as a matter of convention. +Example address value: `"0x0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a"`. + +> [!NOTE] +> Duplicate addresses as well as malformed addresses are detected and logged. + +> [!TIP] +> Application addresses are case insensitive even though Redis values are not. +> So, even though `"0x00000000000000000000000000000000deadbeef"` and `"0x00000000000000000000000000000000DeadBeef"` may belong to the same Redis Set, they are considered to be the same application address and one of them will be identified as a duplicate. + +You may update the contents of `experimental-dapp-addresses-config` at any time, the claimer will adjust accordingly. -The list of addresses can be empty at any time, - the claimer will wait until an application address is added to the set to resume operations. +The set of addresses may be emptied at any time as well, + the claimer will wait until an application address is added to the set before resuming operations.