Add support for wildcards in packet filter (#1931)

* Add filter match support * Implement AsRef<str> for PortId and ChannelId * Fix Deserialize validation for filters * Implement regex support * Add FIXME * Fix clippy errors * Fix regex validation * Refactoring and renaming * Update config.toml * Add unclog entry * Add documentation for `PacketFilter` and associated types. * Fix deserialize impl for FilterPatterns * Rename filter_match.rs to filter_pattern.rs * Add `FromStr` and remove `Into<String>` for `ics04_channel::Version` * Parse channel `Version` directly in `create channel` command * Use `Version` instead of `String` in `Channel::new` * Add `ChannelVersionOverride` trait to override the channel version in integration tests * Fix warning * WIP: Write an integration test that creates a ICA channel * Add facility to modify genesis file * Rename some things * Add packet filtering snippet to relayer config example. * Add test for deserializing packet filter policy * Add packet filter policy serialization test. * Fix serialize_packet_filter_policy test * Cleanup config tests * Fix TOML serialization of ChannelFilters and FilterPattern * Rename some methods * Move filter-related code into its own module * Uncomment packet filter policy in relayer example config * Fix link in doc comment * Remove some unnecessary clones * Add test for iter_exact and impl PartialEq for Wildcard * Get iter_exact test passing * Add allow and deny filter tests * Disable ICA filter test * Rename `spec` to `filters` * Update `regex` crate to v1.5.5 * Update lockfiles * Revert "Add facility to modify genesis file" This reverts commit dd803a9. * Comment out ICA filter test * Add `ChannelFilters::new` * Add working ICA filter integration test * Add test for when ICA channels are disallowed * Apply the channel filter to channel handshakes as well * Enable ICA filter test on CI * Update Nix path to icad * Refactor ICA test * Add changelog entry * Extend ICA filter test with ICA transfer * Add facility for modifying the genesis file * Modify genesis file to allow MsgSend messages over ICA * Cleanup * Add comments * Re-enable ordered channel test * Remove top-level unrelated file * Use `CHAIN_COMMAND_PATH` env variable to run the test with icad instead of gaiad without having to hard code the path * Move ICA driver methods into their own module as functions * Change signature of `assert_eventual_wallet_amount` to take wallet address * Add some doc comments * Rename `ica-filter` test feature and module to `ica` * Remove duplicate changelog entry * Add back missing file Co-authored-by: Sean Chen <[email protected]> Co-authored-by: Romain Ruetschi <[email protected]> Co-authored-by: Soares Chen <[email protected]>
informalsystems · Mar 22, 2022 · bc989c3 · bc989c3
1 parent 1481a16
commit bc989c3
Show file tree

Hide file tree

Showing 42 changed files with 1,264 additions and 183 deletions.
diff --git a/.changelog/unreleased/features/ibc-relayer/1927-packet-filtering-wildcards.md b/.changelog/unreleased/features/ibc-relayer/1927-packet-filtering-wildcards.md
@@ -0,0 +1,3 @@
+- Add support for wildcards in port and channel identifiers in the packet filter configuration,
+  which enable operators to filter ICA channels based on the port prefix
+  ([#1927](https://github.com/informalsystems/ibc-rs/issues/1927))
diff --git a/.github/workflows/integration.yaml b/.github/workflows/integration.yaml
@@ -104,3 +104,34 @@ jobs:
           nix shell github:informalsystems/cosmos.nix/gaia-ordered#gaia6-ordered -c cargo \
             test -p ibc-integration-test --features ordered --no-fail-fast -- \
             --nocapture --test-threads=1 test_ordered_channel
+
+  ica-filter-test:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: cachix/install-nix-action@v15
+        with:
+          install_url: https://nixos-nix-install-tests.cachix.org/serve/vij683ly7sl95nnhb67bdjjfabclr85m/install
+          install_options: '--tarball-url-prefix https://nixos-nix-install-tests.cachix.org/serve'
+          extra_nix_config: |
+            experimental-features = nix-command flakes
+      - uses: cachix/cachix-action@v10
+        with:
+          name: cosmos
+      - uses: actions-rs/toolchain@v1
+        with:
+          toolchain: stable
+          override: true
+      - uses: Swatinem/rust-cache@v1
+      - uses: actions-rs/cargo@v1
+        with:
+          command: test
+          args: -p ibc-integration-test --no-fail-fast --no-run
+      - env:
+          RUST_LOG: info
+          RUST_BACKTRACE: 1
+          CHAIN_COMMAND_PATH: icad
+        run: |
+          nix shell github:informalsystems/cosmos.nix#ica -c cargo \
+            test -p ibc-integration-test --features ica --no-fail-fast -- \
+            --nocapture --test-threads=1 test_ica_filter
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/config.toml b/config.toml
@@ -206,13 +206,15 @@ memo_prefix = ''
 # 1. `policy` - one of two types are supported:
 #       - 'allow': permit relaying _only on_ the port/channel id in the list below,
 #       - 'deny': permit relaying on any channel _except for_ the list below.
-# 2. `list` - the list of channels specified by the port and channel identifiers.
+# 2. `list` - the list of channels specified by the port and channel identifiers. Optionally, each element may also be a
+#       'wildcard', for eg. 'ica*' to match all identifiers starting with 'ica' or '*' to match all identifiers.
 #
 # Example configuration of a channel filter, denying packet relaying on channel with port ID 'transfer' and channel ID 'channel-0':
 #
 # [chains.packet_filter]
-# policy = 'deny'
+# policy = 'allow'
 # list = [
+#   ['ica*', '*'],
 #   ['transfer', 'channel-0'],
 # ]
 

diff --git a/modules/src/core/ics04_channel/channel.rs b/modules/src/core/ics04_channel/channel.rs
@@ -67,7 +67,7 @@ impl From<IdentifiedChannelEnd> for RawIdentifiedChannel {
                 .iter()
                 .map(|v| v.as_str().to_string())
                 .collect(),
-            version: value.channel_end.version.into(),
+            version: value.channel_end.version.to_string(),
             port_id: value.port_id.to_string(),
             channel_id: value.channel_id.to_string(),
         }
@@ -146,7 +146,7 @@ impl From<ChannelEnd> for RawChannel {
                 .iter()
                 .map(|v| v.as_str().to_string())
                 .collect(),
-            version: value.version.into(),
+            version: value.version.to_string(),
         }
     }
 }

diff --git a/modules/src/core/ics04_channel/msgs/chan_open_ack.rs b/modules/src/core/ics04_channel/msgs/chan_open_ack.rs
@@ -95,7 +95,7 @@ impl From<MsgChannelOpenAck> for RawMsgChannelOpenAck {
             port_id: domain_msg.port_id.to_string(),
             channel_id: domain_msg.channel_id.to_string(),
             counterparty_channel_id: domain_msg.counterparty_channel_id.to_string(),
-            counterparty_version: domain_msg.counterparty_version.into(),
+            counterparty_version: domain_msg.counterparty_version.to_string(),
             proof_try: domain_msg.proofs.object_proof().clone().into(),
             proof_height: Some(domain_msg.proofs.height().into()),
             signer: domain_msg.signer.to_string(),

diff --git a/modules/src/core/ics04_channel/msgs/chan_open_try.rs b/modules/src/core/ics04_channel/msgs/chan_open_try.rs
@@ -122,7 +122,7 @@ impl From<MsgChannelOpenTry> for RawMsgChannelOpenTry {
                 .previous_channel_id
                 .map_or_else(|| "".to_string(), |v| v.as_str().to_string()),
             channel: Some(domain_msg.channel.into()),
-            counterparty_version: domain_msg.counterparty_version.into(),
+            counterparty_version: domain_msg.counterparty_version.to_string(),
             proof_init: domain_msg.proofs.object_proof().clone().into(),
             proof_height: Some(domain_msg.proofs.height().into()),
             signer: domain_msg.signer.to_string(),

diff --git a/modules/src/core/ics04_channel/version.rs b/modules/src/core/ics04_channel/version.rs
@@ -2,7 +2,9 @@
 //! version field of a channel end.
 //!
 
+use core::convert::Infallible;
 use core::fmt;
+use core::str::FromStr;
 use serde_derive::{Deserialize, Serialize};
 
 use crate::applications::ics20_fungible_token_transfer;
@@ -17,31 +19,30 @@ use crate::prelude::*;
 pub struct Version(String);
 
 impl Version {
-    pub fn ics20() -> Self {
-        Self(ics20_fungible_token_transfer::VERSION.to_string())
+    pub fn new(v: String) -> Self {
+        Self(v)
     }
 
-    pub fn empty() -> Self {
-        Self("".to_string())
+    pub fn ics20() -> Self {
+        Self::new(ics20_fungible_token_transfer::VERSION.to_string())
     }
-}
 
-impl From<Version> for String {
-    fn from(domain_version: Version) -> Self {
-        domain_version.0
+    pub fn empty() -> Self {
+        Self::new("".to_string())
     }
 }
 
 impl From<String> for Version {
-    fn from(raw_version: String) -> Self {
-        // Version validation: nothing specific.
-        Self(raw_version)
+    fn from(s: String) -> Self {
+        Self::new(s)
     }
 }
 
-impl From<&str> for Version {
-    fn from(raw_version: &str) -> Self {
-        Self(raw_version.into())
+impl FromStr for Version {
+    type Err = Infallible;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        Ok(Self::new(s.to_string()))
     }
 }
 

diff --git a/modules/src/core/ics24_host/identifier.rs b/modules/src/core/ics24_host/identifier.rs
@@ -324,6 +324,12 @@ impl FromStr for PortId {
     }
 }
 
+impl AsRef<str> for PortId {
+    fn as_ref(&self) -> &str {
+        self.0.as_str()
+    }
+}
+
 impl Default for PortId {
     fn default() -> Self {
         "defaultPort".to_string().parse().unwrap()
@@ -380,6 +386,12 @@ impl FromStr for ChannelId {
     }
 }
 
+impl AsRef<str> for ChannelId {
+    fn as_ref(&self) -> &str {
+        self.0.as_str()
+    }
+}
+
 impl Default for ChannelId {
     fn default() -> Self {
         Self::new(0)

diff --git a/relayer-cli/src/commands/create/channel.rs b/relayer-cli/src/commands/create/channel.rs
@@ -4,6 +4,7 @@ use abscissa_core::{Command, Runnable};
 use ibc::core::ics02_client::client_state::ClientState;
 use ibc::core::ics03_connection::connection::IdentifiedConnectionEnd;
 use ibc::core::ics04_channel::channel::Order;
+use ibc::core::ics04_channel::Version;
 use ibc::core::ics24_host::identifier::{ChainId, ConnectionId, PortId};
 use ibc::Height;
 use ibc_relayer::chain::handle::ChainHandle;
@@ -63,7 +64,7 @@ pub struct CreateChannelCommand {
         alias = "version",
         help = "the version for the new channel"
     )]
-    version: Option<String>,
+    version: Option<Version>,
 }
 
 impl Runnable for CreateChannelCommand {

diff --git a/relayer/Cargo.toml b/relayer/Cargo.toml
@@ -61,6 +61,7 @@ semver = "1.0"
 uint = "0.9"
 humantime = "2.1.0"
 nanoid = "0.4.0"
+regex = "1.5.5"
 moka = "0.7.2"
 
 [dependencies.num-bigint]

diff --git a/relayer/src/channel.rs b/relayer/src/channel.rs
@@ -120,6 +120,10 @@ impl<Chain: ChainHandle> ChannelSide<Chain> {
         self.channel_id.as_ref()
     }
 
+    pub fn version(&self) -> Option<&Version> {
+        self.version.as_ref()
+    }
+
     pub fn map_chain<ChainB: ChainHandle>(
         self,
         mapper: impl Fn(Chain) -> ChainB,
@@ -151,7 +155,7 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Channel<ChainA, ChainB> {
         ordering: Order,
         a_port: PortId,
         b_port: PortId,
-        version: Option<String>,
+        version: Option<Version>,
     ) -> Result<Self, ChannelError> {
         let src_connection_id = connection
             .src_connection_id()
@@ -160,9 +164,6 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Channel<ChainA, ChainB> {
             .dst_connection_id()
             .ok_or_else(|| ChannelError::missing_local_connection(connection.dst_chain().id()))?;
 
-        // Convert the raw version into our domain type.
-        let domain_version = version.map(Into::into);
-
         let mut channel = Self {
             ordering,
             a_side: ChannelSide::new(
@@ -171,15 +172,15 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> Channel<ChainA, ChainB> {
                 src_connection_id.clone(),
                 a_port,
                 Default::default(),
-                domain_version.clone(),
+                version.clone(),
             ),
             b_side: ChannelSide::new(
                 connection.dst_chain(),
                 connection.dst_client_id().clone(),
                 dst_connection_id.clone(),
                 b_port,
                 Default::default(),
-                domain_version,
+                version,
             ),
             connection_delay: connection.delay_period,
         };

diff --git a/relayer/src/config.rs b/relayer/src/config.rs
@@ -1,14 +1,12 @@
 //! Relayer configuration
 
-mod error;
-mod proof_specs;
-
+pub mod error;
+pub mod filter;
+pub mod proof_specs;
 pub mod types;
 
 use alloc::collections::BTreeMap;
-use alloc::collections::BTreeSet;
 use core::{fmt, time::Duration};
-use itertools::Itertools;
 use std::sync::{Arc, RwLock};
 use std::{fs, fs::File, io::Write, path::Path};
 
@@ -24,6 +22,8 @@ use crate::keyring::Store;
 
 pub use error::Error;
 
+pub use filter::PacketFilter;
+
 #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
 pub struct GasPrice {
     pub price: f64,
@@ -42,64 +42,6 @@ impl fmt::Display for GasPrice {
     }
 }
 
-#[derive(Clone, Debug, Serialize, Deserialize)]
-#[serde(
-    rename_all = "lowercase",
-    tag = "policy",
-    content = "list",
-    deny_unknown_fields
-)]
-pub enum PacketFilter {
-    Allow(ChannelsSpec),
-    Deny(ChannelsSpec),
-    AllowAll,
-}
-
-impl Default for PacketFilter {
-    /// By default, allows all channels & ports.
-    fn default() -> Self {
-        Self::AllowAll
-    }
-}
-
-impl PacketFilter {
-    /// Returns true if the packets can be relayed on the channel with [`PortId`] and [`ChannelId`],
-    /// false otherwise.
-    pub fn is_allowed(&self, port_id: &PortId, channel_id: &ChannelId) -> bool {
-        match self {
-            PacketFilter::Allow(spec) => spec.contains(&(port_id.clone(), channel_id.clone())),
-            PacketFilter::Deny(spec) => !spec.contains(&(port_id.clone(), channel_id.clone())),
-            PacketFilter::AllowAll => true,
-        }
-    }
-}
-
-#[derive(Clone, Debug, Default, Serialize, Deserialize)]
-#[serde(deny_unknown_fields)]
-pub struct ChannelsSpec(BTreeSet<(PortId, ChannelId)>);
-
-impl ChannelsSpec {
-    pub fn contains(&self, channel_port: &(PortId, ChannelId)) -> bool {
-        self.0.contains(channel_port)
-    }
-
-    pub fn iter(&self) -> impl Iterator<Item = &(PortId, ChannelId)> {
-        self.0.iter()
-    }
-}
-
-impl fmt::Display for ChannelsSpec {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(
-            f,
-            "{}",
-            self.iter()
-                .map(|(pid, cid)| format!("{}/{}", pid, cid))
-                .join(", ")
-        )
-    }
-}
-
 /// Defaults for various fields
 pub mod default {
     use super::*;
@@ -465,9 +407,9 @@ mod tests {
             "/tests/config/fixtures/relayer_conf_example.toml"
         );
 
-        let config = load(path);
-        println!("{:?}", config);
-        assert!(config.is_ok());
+        let config = load(path).expect("could not parse config");
+
+        dbg!(config);
     }
 
     #[test]