From 0ed3c190651e4a2c07ffe304af95ff5756d13e7d Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Thu, 5 Sep 2024 11:47:12 +0200 Subject: [PATCH] feat(proto, core, sequencer)!: permit bech32 compatible addresses (#1425) ## Summary Enables bech32 compatible addresses when necessary. ## Background Some IBC enabled chains enforce that ics20 transfer packets must contain bech32 compatible only (see https://github.com/astriaorg/astria/issues/1407 for more background). This patch adds a `ibc_compat` prefix to sequencer's genesis and a new field `bool Ics20Withdrawal.use_compat_address` to the withdrawal action to request a bech32 compatible address format when interacting with such chains. ## Changes - Add field `astria.protocol.transactions.v1alpha1.Ics20Withdrawal.use_compat_address` - Add field `astria.protocol.genesis.v1alpha1.AddressPrefixes.ibc_compat` - Rewrite the `Address` domain type to be generic over the format; this allows bech32 and bech32m addresses when parsing and formatting - Note that all Astria domain types containing addresses now require `Address` with `Bech32m` also being set as the default type is the parameter is absent - this now enforces that all Astria addresses in Astria's domain types *must* be bech32m compatible (this was not enforced before because `bech32::decode` does not enforce either checksum as of `bech32@v0.11`). - When executing an `astria.protocol.transactions.v1alpha1.Ics20Withdrawal` with field `use_compat_address == true`, sequencer will now construct a bech32 compatible address using the value stored at `prefixes/ibc-compat` in its state. - When refunding a rollup due to a failed withdrawal, sequencer will now attempt to parse the sender as either a standard (non-compat) bech32m address, or a compat bech32 address. - If a compat bech32 address was found during a refund, sequencer will convert it to a bech32m address with its base prefix for further processing. This ensures that the compat addresses are only ever constructed for outgoing and incoming fungible token packets. ## Testing + Updated sequencer tests for ics20 refunds for both compat and base/non-compat senders. + Updated all snapshot tests + Added a snapshot test for `Address` + Added unit tests for address parsing (to ensure that bech32 addresses cannot be parsaed as bech32m and vice versa) + Updated chart sequencer genesis to run smoke tests + IBC tests for noble are still work in progress and outside the scope of this PR. ## Breaking Changelist - Sequencer gained a new genesis value (the ibc compat prefix), changing the app hash ## Related Issues Closes https://github.com/astriaorg/astria/issues/1407 --- charts/sequencer/Chart.yaml | 2 +- .../files/cometbft/config/genesis.json | 3 +- charts/sequencer/values.yaml | 1 + crates/astria-bridge-contracts/src/lib.rs | 3 + .../helpers/test_bridge_withdrawer.rs | 2 + crates/astria-cli/src/commands/sequencer.rs | 3 +- .../astria.protocol.genesis.v1alpha1.rs | 5 + .../astria.protocol.genesis.v1alpha1.serde.rs | 18 + .../astria.protocol.transactions.v1alpha1.rs | 5 + ...ia.protocol.transactions.v1alpha1.serde.rs | 18 + crates/astria-core/src/primitive/v1/mod.rs | 344 +++++++++++++++--- ...re__primitive__v1__tests__snapshots-2.snap | 5 + ...a1__tests__genesis_state_is_unchanged.snap | 3 +- .../src/protocol/genesis/v1alpha1.rs | 40 +- .../protocol/transaction/v1alpha1/action.rs | 23 +- .../src/genesis_example.rs | 9 +- .../astria-sequencer/src/address/state_ext.rs | 64 ++-- crates/astria-sequencer/src/app/mod.rs | 7 +- ...ransaction_with_every_action_snapshot.snap | 62 ++-- ..._changes__app_finalize_block_snapshot.snap | 60 +-- ...reaking_changes__app_genesis_snapshot.snap | 58 +-- crates/astria-sequencer/src/app/test_utils.rs | 10 +- .../src/authority/state_ext.rs | 2 +- .../src/bridge/bridge_lock_action.rs | 2 +- .../src/bridge/bridge_sudo_change_action.rs | 4 +- .../src/bridge/bridge_unlock_action.rs | 62 +--- crates/astria-sequencer/src/bridge/query.rs | 2 +- .../src/ibc/ics20_transfer.rs | 275 +++++++++++--- .../src/ibc/ics20_withdrawal.rs | 51 ++- crates/astria-sequencer/src/ibc/state_ext.rs | 8 +- .../astria-sequencer/src/service/info/mod.rs | 2 +- crates/astria-sequencer/src/test_utils.rs | 11 + .../src/transaction/checks.rs | 5 +- .../protocol/genesis/v1alpha1/types.proto | 4 + .../transactions/v1alpha1/types.proto | 5 + 35 files changed, 846 insertions(+), 332 deletions(-) create mode 100644 crates/astria-core/src/primitive/v1/snapshots/astria_core__primitive__v1__tests__snapshots-2.snap diff --git a/charts/sequencer/Chart.yaml b/charts/sequencer/Chart.yaml index 99346c5cbc..70e4b040dd 100644 --- a/charts/sequencer/Chart.yaml +++ b/charts/sequencer/Chart.yaml @@ -15,7 +15,7 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 0.22.0 +version: 0.22.1 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to # follow Semantic Versioning. They should reflect the version the application is using. diff --git a/charts/sequencer/files/cometbft/config/genesis.json b/charts/sequencer/files/cometbft/config/genesis.json index 9e266902c2..8caf12aa32 100644 --- a/charts/sequencer/files/cometbft/config/genesis.json +++ b/charts/sequencer/files/cometbft/config/genesis.json @@ -23,7 +23,8 @@ "outbound_ics20_transfers_enabled": {{ .Values.genesis.ibc.outboundEnabled }} }, "address_prefixes": { - "base": "{{ .Values.genesis.addressPrefixes.base }}" + "base": "{{ .Values.genesis.addressPrefixes.base }}", + "ibcCompat": "{{ .Values.genesis.addressPrefixes.ibcCompat }}" }, "accounts": [ {{- range $index, $value := .Values.genesis.genesisAccounts }} diff --git a/charts/sequencer/values.yaml b/charts/sequencer/values.yaml index fab0c8db03..89dfb9accc 100644 --- a/charts/sequencer/values.yaml +++ b/charts/sequencer/values.yaml @@ -26,6 +26,7 @@ genesis: genesisTime: "" # '2023-09-22T17:22:35.092832Z' addressPrefixes: base: "astria" + ibcCompat: "astriacompat" authoritySudoAddress: "" nativeAssetBaseDenomination: nria allowedFeeAssets: [] diff --git a/crates/astria-bridge-contracts/src/lib.rs b/crates/astria-bridge-contracts/src/lib.rs index 157e809211..79c821167a 100644 --- a/crates/astria-bridge-contracts/src/lib.rs +++ b/crates/astria-bridge-contracts/src/lib.rs @@ -420,6 +420,9 @@ where timeout_time: timeout_in_5_min(), source_channel, bridge_address: Some(self.bridge_address), + // FIXME: this needs a way to determine when to use compat address + // https://github.com/astriaorg/astria/issues/1424 + use_compat_address: false, }; Ok(Action::Ics20Withdrawal(action)) } diff --git a/crates/astria-bridge-withdrawer/tests/blackbox/helpers/test_bridge_withdrawer.rs b/crates/astria-bridge-withdrawer/tests/blackbox/helpers/test_bridge_withdrawer.rs index 0847aa77cc..a301a26b69 100644 --- a/crates/astria-bridge-withdrawer/tests/blackbox/helpers/test_bridge_withdrawer.rs +++ b/crates/astria-bridge-withdrawer/tests/blackbox/helpers/test_bridge_withdrawer.rs @@ -412,6 +412,7 @@ impl From for SubsetOfIcs20Withdrawal { fee_asset, memo, bridge_address, + use_compat_address: _use_compat_address, } = value; Self { amount, @@ -464,6 +465,7 @@ pub fn make_ics20_withdrawal_action(receipt: &TransactionReceipt) -> Action { timeout_time, source_channel: "channel-0".parse().unwrap(), bridge_address: Some(default_bridge_address()), + use_compat_address: false, }; Action::Ics20Withdrawal(inner) diff --git a/crates/astria-cli/src/commands/sequencer.rs b/crates/astria-cli/src/commands/sequencer.rs index 63311c7df5..c7c601ee62 100644 --- a/crates/astria-cli/src/commands/sequencer.rs +++ b/crates/astria-cli/src/commands/sequencer.rs @@ -2,6 +2,7 @@ use astria_core::{ crypto::SigningKey, primitive::v1::{ Address, + Bech32m, ADDRESS_LEN, }, protocol::transaction::v1alpha1::{ @@ -167,7 +168,7 @@ pub(crate) fn make_bech32m(args: &Bech32mAddressArgs) -> eyre::Result<()> { use hex::FromHex as _; let bytes = <[u8; ADDRESS_LEN]>::from_hex(&args.bytes) .wrap_err("failed decoding provided hex bytes")?; - let address = Address::builder() + let address = Address::::builder() .array(bytes) .prefix(&args.prefix) .try_build() diff --git a/crates/astria-core/src/generated/astria.protocol.genesis.v1alpha1.rs b/crates/astria-core/src/generated/astria.protocol.genesis.v1alpha1.rs index 17ccce5d82..cf46eec58d 100644 --- a/crates/astria-core/src/generated/astria.protocol.genesis.v1alpha1.rs +++ b/crates/astria-core/src/generated/astria.protocol.genesis.v1alpha1.rs @@ -53,8 +53,13 @@ impl ::prost::Name for Account { #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct AddressPrefixes { + /// The base prefix used for most Astria Sequencer addresses. #[prost(string, tag = "1")] pub base: ::prost::alloc::string::String, + /// The prefix used for sending ics20 transfers to IBC chains + /// that enforce a bech32 format of the packet sender. + #[prost(string, tag = "2")] + pub ibc_compat: ::prost::alloc::string::String, } impl ::prost::Name for AddressPrefixes { const NAME: &'static str = "AddressPrefixes"; diff --git a/crates/astria-core/src/generated/astria.protocol.genesis.v1alpha1.serde.rs b/crates/astria-core/src/generated/astria.protocol.genesis.v1alpha1.serde.rs index 4636c57237..3ae22e9461 100644 --- a/crates/astria-core/src/generated/astria.protocol.genesis.v1alpha1.serde.rs +++ b/crates/astria-core/src/generated/astria.protocol.genesis.v1alpha1.serde.rs @@ -117,10 +117,16 @@ impl serde::Serialize for AddressPrefixes { if !self.base.is_empty() { len += 1; } + if !self.ibc_compat.is_empty() { + len += 1; + } let mut struct_ser = serializer.serialize_struct("astria.protocol.genesis.v1alpha1.AddressPrefixes", len)?; if !self.base.is_empty() { struct_ser.serialize_field("base", &self.base)?; } + if !self.ibc_compat.is_empty() { + struct_ser.serialize_field("ibcCompat", &self.ibc_compat)?; + } struct_ser.end() } } @@ -132,11 +138,14 @@ impl<'de> serde::Deserialize<'de> for AddressPrefixes { { const FIELDS: &[&str] = &[ "base", + "ibc_compat", + "ibcCompat", ]; #[allow(clippy::enum_variant_names)] enum GeneratedField { Base, + IbcCompat, } impl<'de> serde::Deserialize<'de> for GeneratedField { fn deserialize(deserializer: D) -> std::result::Result @@ -159,6 +168,7 @@ impl<'de> serde::Deserialize<'de> for AddressPrefixes { { match value { "base" => Ok(GeneratedField::Base), + "ibcCompat" | "ibc_compat" => Ok(GeneratedField::IbcCompat), _ => Err(serde::de::Error::unknown_field(value, FIELDS)), } } @@ -179,6 +189,7 @@ impl<'de> serde::Deserialize<'de> for AddressPrefixes { V: serde::de::MapAccess<'de>, { let mut base__ = None; + let mut ibc_compat__ = None; while let Some(k) = map_.next_key()? { match k { GeneratedField::Base => { @@ -187,10 +198,17 @@ impl<'de> serde::Deserialize<'de> for AddressPrefixes { } base__ = Some(map_.next_value()?); } + GeneratedField::IbcCompat => { + if ibc_compat__.is_some() { + return Err(serde::de::Error::duplicate_field("ibcCompat")); + } + ibc_compat__ = Some(map_.next_value()?); + } } } Ok(AddressPrefixes { base: base__.unwrap_or_default(), + ibc_compat: ibc_compat__.unwrap_or_default(), }) } } diff --git a/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.rs b/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.rs index a65b8a8b28..c83dd64f57 100644 --- a/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.rs +++ b/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.rs @@ -222,6 +222,11 @@ pub struct Ics20Withdrawal { pub bridge_address: ::core::option::Option< super::super::super::primitive::v1::Address, >, + /// whether to use a bech32-compatible format of the `.return_address` when generating + /// fungible token packets (as opposed to Astria-native bech32m addresses). This is + /// necessary for chains like noble which enforce a strict bech32 format. + #[prost(bool, tag = "11")] + pub use_compat_address: bool, } impl ::prost::Name for Ics20Withdrawal { const NAME: &'static str = "Ics20Withdrawal"; diff --git a/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.serde.rs b/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.serde.rs index f6bad8ea54..5e11f90655 100644 --- a/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.serde.rs +++ b/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.serde.rs @@ -1321,6 +1321,9 @@ impl serde::Serialize for Ics20Withdrawal { if self.bridge_address.is_some() { len += 1; } + if self.use_compat_address { + len += 1; + } let mut struct_ser = serializer.serialize_struct("astria.protocol.transactions.v1alpha1.Ics20Withdrawal", len)?; if let Some(v) = self.amount.as_ref() { struct_ser.serialize_field("amount", v)?; @@ -1353,6 +1356,9 @@ impl serde::Serialize for Ics20Withdrawal { if let Some(v) = self.bridge_address.as_ref() { struct_ser.serialize_field("bridgeAddress", v)?; } + if self.use_compat_address { + struct_ser.serialize_field("useCompatAddress", &self.use_compat_address)?; + } struct_ser.end() } } @@ -1380,6 +1386,8 @@ impl<'de> serde::Deserialize<'de> for Ics20Withdrawal { "memo", "bridge_address", "bridgeAddress", + "use_compat_address", + "useCompatAddress", ]; #[allow(clippy::enum_variant_names)] @@ -1394,6 +1402,7 @@ impl<'de> serde::Deserialize<'de> for Ics20Withdrawal { FeeAsset, Memo, BridgeAddress, + UseCompatAddress, } impl<'de> serde::Deserialize<'de> for GeneratedField { fn deserialize(deserializer: D) -> std::result::Result @@ -1425,6 +1434,7 @@ impl<'de> serde::Deserialize<'de> for Ics20Withdrawal { "feeAsset" | "fee_asset" => Ok(GeneratedField::FeeAsset), "memo" => Ok(GeneratedField::Memo), "bridgeAddress" | "bridge_address" => Ok(GeneratedField::BridgeAddress), + "useCompatAddress" | "use_compat_address" => Ok(GeneratedField::UseCompatAddress), _ => Err(serde::de::Error::unknown_field(value, FIELDS)), } } @@ -1454,6 +1464,7 @@ impl<'de> serde::Deserialize<'de> for Ics20Withdrawal { let mut fee_asset__ = None; let mut memo__ = None; let mut bridge_address__ = None; + let mut use_compat_address__ = None; while let Some(k) = map_.next_key()? { match k { GeneratedField::Amount => { @@ -1518,6 +1529,12 @@ impl<'de> serde::Deserialize<'de> for Ics20Withdrawal { } bridge_address__ = map_.next_value()?; } + GeneratedField::UseCompatAddress => { + if use_compat_address__.is_some() { + return Err(serde::de::Error::duplicate_field("useCompatAddress")); + } + use_compat_address__ = Some(map_.next_value()?); + } } } Ok(Ics20Withdrawal { @@ -1531,6 +1548,7 @@ impl<'de> serde::Deserialize<'de> for Ics20Withdrawal { fee_asset: fee_asset__.unwrap_or_default(), memo: memo__.unwrap_or_default(), bridge_address: bridge_address__, + use_compat_address: use_compat_address__.unwrap_or_default(), }) } } diff --git a/crates/astria-core/src/primitive/v1/mod.rs b/crates/astria-core/src/primitive/v1/mod.rs index bf88007ec9..e9e5ce2d55 100644 --- a/crates/astria-core/src/primitive/v1/mod.rs +++ b/crates/astria-core/src/primitive/v1/mod.rs @@ -1,7 +1,10 @@ pub mod asset; pub mod u128; -use std::str::FromStr; +use std::{ + marker::PhantomData, + str::FromStr, +}; use base64::{ display::Base64Display, @@ -241,8 +244,8 @@ pub struct IncorrectRollupIdLength { pub struct AddressError(AddressErrorKind); impl AddressError { - fn bech32m_decode(source: bech32::DecodeError) -> Self { - Self(AddressErrorKind::Bech32mDecode { + fn decode(source: bech32::primitives::decode::CheckedHrpstringError) -> Self { + Self(AddressErrorKind::Decode { source, }) } @@ -262,8 +265,10 @@ impl AddressError { #[derive(Debug, thiserror::Error, PartialEq)] enum AddressErrorKind { - #[error("failed decoding provided bech32m string")] - Bech32mDecode { source: bech32::DecodeError }, + #[error("failed decoding provided string")] + Decode { + source: bech32::primitives::decode::CheckedHrpstringError, + }, #[error("expected an address of 20 bytes, got `{received}`")] IncorrectAddressLength { received: usize }, #[error("the provided prefix was not a valid bech32 human readable prefix")] @@ -274,33 +279,50 @@ enum AddressErrorKind { pub struct NoBytes; pub struct NoPrefix; -pub struct WithBytes<'a>(BytesInner<'a>); -enum BytesInner<'a> { +pub struct WithBytes<'a, I>(WithBytesInner<'a, I>); +enum WithBytesInner<'a, I> { Array([u8; ADDRESS_LEN]), + Iter(I), Slice(std::borrow::Cow<'a, [u8]>), } pub struct WithPrefix<'a>(std::borrow::Cow<'a, str>); -pub struct AddressBuilder { +pub struct NoBytesIter; + +impl Iterator for NoBytesIter { + type Item = u8; + + fn next(&mut self) -> Option { + None + } +} + +pub struct AddressBuilder { bytes: TBytes, prefix: TPrefix, + format: PhantomData, } -impl AddressBuilder { +impl AddressBuilder { const fn new() -> Self { Self { bytes: NoBytes, prefix: NoPrefix, + format: PhantomData, } } } -impl AddressBuilder { +impl AddressBuilder { #[must_use = "the builder must be built to construct an address to be useful"] - pub fn array(self, array: [u8; ADDRESS_LEN]) -> AddressBuilder, TPrefix> { + pub fn array( + self, + array: [u8; ADDRESS_LEN], + ) -> AddressBuilder, TPrefix> { AddressBuilder { - bytes: WithBytes(BytesInner::Array(array)), + bytes: WithBytes(WithBytesInner::Array(array)), prefix: self.prefix, + format: self.format, } } @@ -308,10 +330,23 @@ impl AddressBuilder { pub fn slice<'a, T: Into>>( self, bytes: T, - ) -> AddressBuilder, TPrefix> { + ) -> AddressBuilder, TPrefix> { + AddressBuilder { + bytes: WithBytes(WithBytesInner::Slice(bytes.into())), + prefix: self.prefix, + format: self.format, + } + } + + #[must_use = "the builder must be built to construct an address to be useful"] + pub fn with_iter>( + self, + iter: T, + ) -> AddressBuilder, TPrefix> { AddressBuilder { - bytes: WithBytes(BytesInner::Slice(bytes.into())), + bytes: WithBytes(WithBytesInner::Iter(iter)), prefix: self.prefix, + format: self.format, } } @@ -324,7 +359,7 @@ impl AddressBuilder { pub fn verification_key( self, key: &crate::crypto::VerificationKey, - ) -> AddressBuilder, TPrefix> { + ) -> AddressBuilder, TPrefix> { let hash = Sha256::digest(key.as_bytes()); let array: [u8; ADDRESS_LEN] = hash[0..ADDRESS_LEN] .try_into() @@ -336,15 +371,19 @@ impl AddressBuilder { pub fn prefix<'a, T: Into>>( self, prefix: T, - ) -> AddressBuilder> { + ) -> AddressBuilder> { AddressBuilder { bytes: self.bytes, prefix: WithPrefix(prefix.into()), + format: self.format, } } } -impl<'a, 'b> AddressBuilder, WithPrefix<'b>> { +impl<'a, 'b, TFormat, TBytesIter> AddressBuilder, WithPrefix<'b>> +where + TBytesIter: IntoIterator, +{ /// Attempts to build an address from the configured prefix and bytes. /// /// # Errors @@ -352,39 +391,132 @@ impl<'a, 'b> AddressBuilder, WithPrefix<'b>> { /// + if the prefix shorter than 1 or longer than 83 characters, or contains characters outside /// 33-126 of ASCII characters. /// + if the provided bytes are not exactly 20 bytes. - pub fn try_build(self) -> Result { + pub fn try_build(self) -> Result, AddressError> { let Self { bytes: WithBytes(bytes), prefix: WithPrefix(prefix), + format, } = self; let bytes = match bytes { - BytesInner::Array(bytes) => bytes, - BytesInner::Slice(bytes) => <[u8; ADDRESS_LEN]>::try_from(bytes.as_ref()) + WithBytesInner::Array(bytes) => bytes, + WithBytesInner::Iter(bytes) => try_collect_to_array(bytes)?, + WithBytesInner::Slice(bytes) => <[u8; ADDRESS_LEN]>::try_from(bytes.as_ref()) .map_err(|_| AddressError::incorrect_address_length(bytes.len()))?, }; let prefix = bech32::Hrp::parse(&prefix).map_err(AddressError::invalid_prefix)?; Ok(Address { bytes, prefix, + format, }) } } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -#[cfg_attr( - feature = "serde", - serde(into = "raw::Address", try_from = "raw::Address") -)] -pub struct Address { +fn try_collect_to_array>( + iter: I, +) -> Result<[u8; ADDRESS_LEN], AddressError> { + let mut arr = [0; ADDRESS_LEN]; + let mut iter = iter.into_iter(); + let mut i = 0; + loop { + if i >= ADDRESS_LEN { + break; + } + let Some(byte) = iter.next() else { + break; + }; + arr[i] = byte; + i = i.saturating_add(1); + } + let items_in_iterator = i.saturating_add(iter.count()); + if items_in_iterator != ADDRESS_LEN { + return Err(AddressError::incorrect_address_length(items_in_iterator)); + } + Ok(arr) +} + +#[derive(Clone, Copy, Debug)] +pub enum Bech32m {} +#[derive(Clone, Copy, Debug)] +pub enum Bech32 {} +#[derive(Clone, Copy, Debug)] +pub enum NoFormat {} + +pub trait Format: private::Sealed { + type Checksum: bech32::Checksum; +} + +impl Format for Bech32m { + type Checksum = bech32::Bech32m; +} + +impl Format for Bech32 { + type Checksum = bech32::Bech32; +} + +impl Format for NoFormat { + type Checksum = bech32::NoChecksum; +} + +mod private { + pub trait Sealed {} + impl Sealed for super::Bech32m {} + impl Sealed for super::Bech32 {} + impl Sealed for super::NoFormat {} +} + +#[derive(Debug, Hash)] +pub struct Address { bytes: [u8; ADDRESS_LEN], prefix: bech32::Hrp, + format: PhantomData, +} + +// The serde impls need to be manually implemented for Address because they +// only work for Address which cannot be expressed using serde +// attributes. +#[cfg(feature = "serde")] +mod _serde_impls { + use serde::de::Error as _; + impl serde::Serialize for super::Address { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + self.to_raw().serialize(serializer) + } + } + impl<'de> serde::Deserialize<'de> for super::Address { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + super::raw::Address::deserialize(deserializer) + .and_then(|raw| raw.try_into().map_err(D::Error::custom)) + } + } +} + +impl Clone for Address { + fn clone(&self) -> Self { + *self + } +} + +impl Copy for Address {} + +impl PartialEq for Address { + fn eq(&self, other: &Self) -> bool { + self.bytes.eq(&other.bytes) && self.prefix.eq(&other.prefix) + } } -impl Address { +impl Eq for Address {} + +impl Address { #[must_use = "the builder must be used to construct an address to be useful"] - pub fn builder() -> AddressBuilder { - AddressBuilder::new() + pub fn builder() -> AddressBuilder { + AddressBuilder::::new() } #[must_use] @@ -397,13 +529,42 @@ impl Address { self.prefix.as_str() } + /// Converts to a new address with the given `prefix`. + /// + /// # Errors + /// Returns an error if an address with `prefix` cannot be constructed. + /// The error conditions for this are the same as for [`AddressBuilder::try_build`]. + pub fn to_prefix(&self, prefix: &str) -> Result { + Self::builder() + .array(self.bytes()) + .prefix(prefix) + .try_build() + } + + /// Converts to a new address with the type argument `OtherFormat`. + /// + /// `OtherFormat` is usually [`Bech32`] or [`Bech32m`]. + #[must_use] + pub fn to_format(&self) -> Address { + Address { + bytes: self.bytes, + prefix: self.prefix, + format: PhantomData, + } + } +} + +impl Address { /// Convert [`Address`] to a [`raw::Address`]. // allow: panics are checked to not happen #[allow(clippy::missing_panics_doc)] #[must_use] pub fn to_raw(&self) -> raw::Address { - let bech32m = bech32::encode_lower::(self.prefix, &self.bytes()) - .expect("should not fail because len(prefix) + len(bytes) <= 63 < BECH32M::CODELENGTH"); + let bech32m = + bech32::encode_lower::<::Checksum>(self.prefix, &self.bytes()) + .expect( + "should not fail because len(prefix) + len(bytes) <= 63 < BECH32M::CODELENGTH", + ); // allow: the field is deprecated, but we must still fill it in #[allow(deprecated)] raw::Address { @@ -429,25 +590,27 @@ impl Address { } } -impl From
for raw::Address { - fn from(value: Address) -> Self { +impl From> for raw::Address { + fn from(value: Address) -> Self { value.into_raw() } } -impl FromStr for Address { +impl FromStr for Address { type Err = AddressError; fn from_str(s: &str) -> Result { - let (hrp, bytes) = bech32::decode(s).map_err(AddressError::bech32m_decode)?; + let checked = bech32::primitives::decode::CheckedHrpstring::new::(s) + .map_err(Self::Err::decode)?; + let hrp = checked.hrp(); Self::builder() - .slice(bytes) + .with_iter(checked.byte_iter()) .prefix(hrp.as_str()) .try_build() } } -impl TryFrom for Address { +impl TryFrom for Address { type Error = AddressError; fn try_from(value: raw::Address) -> Result { @@ -455,20 +618,30 @@ impl TryFrom for Address { } } -impl std::fmt::Display for Address { +impl std::fmt::Display for Address { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { use bech32::EncodeError; - match bech32::encode_lower_to_fmt::(f, self.prefix, &self.bytes()) { + match bech32::encode_lower_to_fmt::(f, self.prefix, &self.bytes()) { Ok(()) => Ok(()), Err(EncodeError::Fmt(err)) => Err(err), Err(err) => panic!( "only formatting errors are valid when encoding astria addresses; all other error \ - variants (only TooLong at of bech32-0.11.0) are guaranteed to not \ - happen:\n{err:?}", + variants (only TooLong as of bech32-0.11.0) are guaranteed to not happen because \ + `Address` is length checked:\n{err:?}", ), } } } +/// Constructs a dummy address from a given `prefix`, otherwise fail. +pub(crate) fn try_construct_dummy_address_from_prefix( + prefix: &str, +) -> Result<(), AddressError> { + Address::::builder() + .array([0u8; ADDRESS_LEN]) + .prefix(prefix) + .try_build() + .map(|_| ()) +} /// Derive a [`merkle::Tree`] from an iterable. /// @@ -494,13 +667,16 @@ mod tests { Address, AddressError, AddressErrorKind, + Bech32m, ADDRESS_LEN, }; + use crate::primitive::v1::Bech32; const ASTRIA_ADDRESS_PREFIX: &str = "astria"; + const ASTRIA_COMPAT_ADDRESS_PREFIX: &str = "astriacompat"; #[track_caller] fn assert_wrong_address_bytes(bad_account: &[u8]) { - let error = Address::builder() + let error = Address::::builder() .slice(bad_account) .prefix(ASTRIA_ADDRESS_PREFIX) .try_build() @@ -528,12 +704,90 @@ mod tests { #[cfg(feature = "serde")] #[test] fn snapshots() { - let address = Address::builder() + use crate::primitive::v1::Bech32; + + let main_address = Address::builder() .array([42; 20]) .prefix(ASTRIA_ADDRESS_PREFIX) .try_build() .unwrap(); - insta::assert_json_snapshot!(address); + insta::assert_json_snapshot!(&main_address); + + let compat_address = main_address + .to_prefix(ASTRIA_COMPAT_ADDRESS_PREFIX) + .unwrap() + .to_format::(); + // We don't allow serializing non bech32m addresses due to + // its impl via the protobuf type. + insta::assert_snapshot!(&compat_address); + } + + #[test] + fn parse_bech32m_address() { + let expected = Address::builder() + .array([42; 20]) + .prefix(ASTRIA_ADDRESS_PREFIX) + .try_build() + .unwrap(); + let actual = expected.to_string().parse::
().unwrap(); + assert_eq!(expected, actual); + } + + #[test] + fn parse_bech32_address() { + let expected = Address::::builder() + .array([42; 20]) + .prefix(ASTRIA_COMPAT_ADDRESS_PREFIX) + .try_build() + .unwrap(); + let actual = expected.to_string().parse::>().unwrap(); + assert_eq!(expected, actual); + } + + #[test] + fn parsing_bech32_address_as_bech32m_fails() { + let expected = Address::::builder() + .array([42; 20]) + .prefix(ASTRIA_COMPAT_ADDRESS_PREFIX) + .try_build() + .unwrap(); + let err = expected + .to_string() + .parse::>() + .expect_err("this must not work"); + match err { + AddressError(AddressErrorKind::Decode { + .. + }) => {} + other => { + panic!( + "expected AddressError(AddressErrorKind::Decode {{ .. }}), but got {other:?}" + ) + } + } + } + + #[test] + fn parsing_bech32m_address_as_bech32_fails() { + let expected = Address::::builder() + .array([42; 20]) + .prefix(ASTRIA_ADDRESS_PREFIX) + .try_build() + .unwrap(); + let err = expected + .to_string() + .parse::>() + .expect_err("this must not work"); + match err { + AddressError(AddressErrorKind::Decode { + .. + }) => {} + other => { + panic!( + "expected AddressError(AddressErrorKind::Decode {{ .. }}), but got {other:?}" + ) + } + } } #[test] diff --git a/crates/astria-core/src/primitive/v1/snapshots/astria_core__primitive__v1__tests__snapshots-2.snap b/crates/astria-core/src/primitive/v1/snapshots/astria_core__primitive__v1__tests__snapshots-2.snap new file mode 100644 index 0000000000..afa9f15103 --- /dev/null +++ b/crates/astria-core/src/primitive/v1/snapshots/astria_core__primitive__v1__tests__snapshots-2.snap @@ -0,0 +1,5 @@ +--- +source: crates/astria-core/src/primitive/v1/mod.rs +expression: "&bech32_address" +--- +astriacompat19g4z52329g4z52329g4z52329g4z52322jspvr diff --git a/crates/astria-core/src/protocol/genesis/snapshots/astria_core__protocol__genesis__v1alpha1__tests__genesis_state_is_unchanged.snap b/crates/astria-core/src/protocol/genesis/snapshots/astria_core__protocol__genesis__v1alpha1__tests__genesis_state_is_unchanged.snap index faa12162a8..3889c03f12 100644 --- a/crates/astria-core/src/protocol/genesis/snapshots/astria_core__protocol__genesis__v1alpha1__tests__genesis_state_is_unchanged.snap +++ b/crates/astria-core/src/protocol/genesis/snapshots/astria_core__protocol__genesis__v1alpha1__tests__genesis_state_is_unchanged.snap @@ -5,7 +5,8 @@ expression: genesis_state() { "chainId": "astria-1", "addressPrefixes": { - "base": "astria" + "base": "astria", + "ibcCompat": "astriacompat" }, "accounts": [ { diff --git a/crates/astria-core/src/protocol/genesis/v1alpha1.rs b/crates/astria-core/src/protocol/genesis/v1alpha1.rs index 2131d1fccc..c5a03b5918 100644 --- a/crates/astria-core/src/protocol/genesis/v1alpha1.rs +++ b/crates/astria-core/src/protocol/genesis/v1alpha1.rs @@ -10,9 +10,11 @@ use crate::{ denom::ParseTracePrefixedError, ParseDenomError, }, + try_construct_dummy_address_from_prefix, Address, AddressError, - ADDRESS_LEN, + Bech32, + Bech32m, }, Protobuf, }; @@ -420,9 +422,26 @@ enum AccountErrorKind { FieldNotSet { name: &'static str }, } +/// The address prefixes used by the Sequencer. +/// +/// All prefixes are guaranteed to be between 1 and 83 bech32 human readable +/// characters in the ASCII range `[33, 126]`. #[derive(Clone, Debug)] pub struct AddressPrefixes { - pub base: String, + base: String, + ibc_compat: String, +} + +impl AddressPrefixes { + #[must_use] + pub fn base(&self) -> &str { + &self.base + } + + #[must_use] + pub fn ibc_compat(&self) -> &str { + &self.ibc_compat + } } impl Protobuf for AddressPrefixes { @@ -432,19 +451,24 @@ impl Protobuf for AddressPrefixes { fn try_from_raw_ref(raw: &Self::Raw) -> Result { let Self::Raw { base, + ibc_compat, } = raw; - try_construct_dummy_address_from_prefix(base).map_err(Self::Error::base)?; + try_construct_dummy_address_from_prefix::(base).map_err(Self::Error::base)?; + try_construct_dummy_address_from_prefix::(ibc_compat).map_err(Self::Error::base)?; Ok(Self { base: base.to_string(), + ibc_compat: ibc_compat.to_string(), }) } fn to_raw(&self) -> Self::Raw { let Self { base, + ibc_compat, } = self; Self::Raw { base: base.clone(), + ibc_compat: ibc_compat.clone(), } } } @@ -609,15 +633,6 @@ enum FeesErrorKind { FieldNotSet { name: &'static str }, } -/// Constructs a dummy address from a given `prefix`, otherwise fail. -fn try_construct_dummy_address_from_prefix(prefix: &str) -> Result<(), AddressError> { - Address::builder() - .array([0u8; ADDRESS_LEN]) - .prefix(prefix) - .try_build() - .map(|_| ()) -} - #[cfg(test)] mod tests { use super::*; @@ -675,6 +690,7 @@ mod tests { ], address_prefixes: Some(raw::AddressPrefixes { base: "astria".into(), + ibc_compat: "astriacompat".into(), }), authority_sudo_address: Some(alice().to_raw()), chain_id: "astria-1".to_string(), diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs index fe23ffd23a..91c747a2f5 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs @@ -7,7 +7,6 @@ use ibc_types::{ IdentifierError, }; use penumbra_ibc::IbcRelay; -use penumbra_proto::penumbra::core::component::ibc::v1::FungibleTokenPacketData; use super::raw; use crate::{ @@ -787,6 +786,11 @@ pub struct Ics20Withdrawal { // if unset, and the transaction sender is a bridge account, the withdrawal is // treated as a bridge withdrawal (ie. the bridge account's withdrawer address is checked). pub bridge_address: Option
, + + // whether to use a bech32-compatible format of the `.return_address` when generating + // fungible token packets (as opposed to Astria-native bech32m addresses). This is + // necessary for chains like noble which enforce a strict bech32 format. + pub use_compat_address: bool, } impl Ics20Withdrawal { @@ -834,17 +838,6 @@ impl Ics20Withdrawal { pub fn memo(&self) -> &str { &self.memo } - - #[must_use] - pub fn to_fungible_token_packet_data(&self) -> FungibleTokenPacketData { - FungibleTokenPacketData { - amount: self.amount.to_string(), - denom: self.denom.to_string(), - sender: self.return_address.to_string(), - receiver: self.destination_chain_address.clone(), - memo: self.memo.clone(), - } - } } impl Protobuf for Ics20Withdrawal { @@ -864,6 +857,7 @@ impl Protobuf for Ics20Withdrawal { fee_asset: self.fee_asset.to_string(), memo: self.memo.clone(), bridge_address: self.bridge_address.as_ref().map(Address::to_raw), + use_compat_address: self.use_compat_address, } } @@ -880,6 +874,7 @@ impl Protobuf for Ics20Withdrawal { fee_asset: self.fee_asset.to_string(), memo: self.memo, bridge_address: self.bridge_address.map(Address::into_raw), + use_compat_address: self.use_compat_address, } } @@ -904,6 +899,7 @@ impl Protobuf for Ics20Withdrawal { fee_asset, memo, bridge_address, + use_compat_address, } = proto; let amount = amount.ok_or(Ics20WithdrawalError::field_not_set("amount"))?; let return_address = Address::try_from_raw( @@ -935,6 +931,7 @@ impl Protobuf for Ics20Withdrawal { .map_err(Ics20WithdrawalError::invalid_fee_asset)?, memo, bridge_address, + use_compat_address, }) } @@ -959,6 +956,7 @@ impl Protobuf for Ics20Withdrawal { fee_asset, memo, bridge_address, + use_compat_address, } = proto; let amount = amount.ok_or(Ics20WithdrawalError::field_not_set("amount"))?; let return_address = Address::try_from_raw( @@ -993,6 +991,7 @@ impl Protobuf for Ics20Withdrawal { .map_err(Ics20WithdrawalError::invalid_fee_asset)?, memo: memo.clone(), bridge_address, + use_compat_address: *use_compat_address, }) } } diff --git a/crates/astria-sequencer-utils/src/genesis_example.rs b/crates/astria-sequencer-utils/src/genesis_example.rs index b28083fb82..680dc0c307 100644 --- a/crates/astria-sequencer-utils/src/genesis_example.rs +++ b/crates/astria-sequencer-utils/src/genesis_example.rs @@ -5,11 +5,13 @@ use std::{ }; use astria_core::{ - generated::protocol::genesis::v1alpha1::IbcParameters, + generated::protocol::genesis::v1alpha1::{ + AddressPrefixes, + IbcParameters, + }, primitive::v1::Address, protocol::genesis::v1alpha1::{ Account, - AddressPrefixes, Fees, GenesisAppState, }, @@ -66,13 +68,14 @@ fn accounts() -> Vec { fn address_prefixes() -> AddressPrefixes { AddressPrefixes { base: "astria".into(), + ibc_compat: "astriacompat".into(), } } fn proto_genesis_state() -> astria_core::generated::protocol::genesis::v1alpha1::GenesisAppState { astria_core::generated::protocol::genesis::v1alpha1::GenesisAppState { accounts: accounts().into_iter().map(Protobuf::into_raw).collect(), - address_prefixes: Some(address_prefixes().into_raw()), + address_prefixes: Some(address_prefixes()), authority_sudo_address: Some(alice().to_raw()), chain_id: "test-1".into(), ibc_sudo_address: Some(alice().to_raw()), diff --git a/crates/astria-sequencer/src/address/state_ext.rs b/crates/astria-sequencer/src/address/state_ext.rs index 924479107a..3016dc36f1 100644 --- a/crates/astria-sequencer/src/address/state_ext.rs +++ b/crates/astria-sequencer/src/address/state_ext.rs @@ -4,7 +4,10 @@ use anyhow::{ Context as _, Result, }; -use astria_core::primitive::v1::Address; +use astria_core::primitive::v1::{ + Address, + Bech32m, +}; use async_trait::async_trait; use cnidarium::{ StateRead, @@ -16,9 +19,13 @@ fn base_prefix_key() -> &'static str { "prefixes/base" } +fn ibc_compat_prefix_key() -> &'static str { + "prefixes/ibc-compat" +} + #[async_trait] pub(crate) trait StateReadExt: StateRead { - async fn ensure_base_prefix(&self, address: &Address) -> anyhow::Result<()> { + async fn ensure_base_prefix(&self, address: &Address) -> anyhow::Result<()> { let prefix = self .get_base_prefix() .await @@ -48,9 +55,21 @@ pub(crate) trait StateReadExt: StateRead { let Some(bytes) = self .get_raw(base_prefix_key()) .await - .context("failed reading address base prefix")? + .context("failed reading address base prefix from state")? else { - bail!("no base prefix found"); + bail!("no base prefix found in state"); + }; + String::from_utf8(bytes).context("prefix retrieved from storage is not valid utf8") + } + + #[instrument(skip_all)] + async fn get_ibc_compat_prefix(&self) -> Result { + let Some(bytes) = self + .get_raw(ibc_compat_prefix_key()) + .await + .context("failed reading address ibc compat prefix from state")? + else { + bail!("no ibc compat prefix found in state") }; String::from_utf8(bytes).context("prefix retrieved from storage is not valid utf8") } @@ -61,28 +80,18 @@ impl StateReadExt for T {} #[async_trait] pub(crate) trait StateWriteExt: StateWrite { #[instrument(skip_all)] - fn put_base_prefix(&mut self, prefix: &str) -> anyhow::Result<()> { - try_construct_dummy_address_from_prefix(prefix) - .context("failed constructing a dummy address from the provided prefix")?; + fn put_base_prefix(&mut self, prefix: &str) { self.put_raw(base_prefix_key().into(), prefix.into()); - Ok(()) + } + + #[instrument(skip_all)] + fn put_ibc_compat_prefix(&mut self, prefix: &str) { + self.put_raw(ibc_compat_prefix_key().into(), prefix.into()); } } impl StateWriteExt for T {} -fn try_construct_dummy_address_from_prefix( - s: &str, -) -> Result<(), astria_core::primitive::v1::AddressError> { - use astria_core::primitive::v1::ADDRESS_LEN; - // construct a dummy address to see if we can construct it; fail otherwise. - Address::builder() - .array([0u8; ADDRESS_LEN]) - .prefix(s) - .try_build() - .map(|_| ()) -} - #[cfg(test)] mod test { use cnidarium::StateDelta; @@ -98,7 +107,20 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix("astria").unwrap(); + state.put_base_prefix("astria"); assert_eq!("astria", &state.get_base_prefix().await.unwrap()); } + + #[tokio::test] + async fn put_and_get_ibc_compat_prefix() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state = StateDelta::new(snapshot); + + state.put_ibc_compat_prefix("astriacompat"); + assert_eq!( + "astriacompat", + &state.get_ibc_compat_prefix().await.unwrap() + ); + } } diff --git a/crates/astria-sequencer/src/app/mod.rs b/crates/astria-sequencer/src/app/mod.rs index e27feb6ec0..4dca9057ca 100644 --- a/crates/astria-sequencer/src/app/mod.rs +++ b/crates/astria-sequencer/src/app/mod.rs @@ -74,7 +74,7 @@ use crate::{ StateReadExt, StateWriteExt as _, }, - address::StateWriteExt as _, + address::StateWriteExt, api_state_ext::StateWriteExt as _, assets::{ StateReadExt as _, @@ -219,9 +219,8 @@ impl App { .try_begin_transaction() .expect("state Arc should not be referenced elsewhere"); - state_tx - .put_base_prefix(&genesis_state.address_prefixes().base) - .context("failed to write base prefix to state")?; + state_tx.put_base_prefix(genesis_state.address_prefixes().base()); + state_tx.put_ibc_compat_prefix(genesis_state.address_prefixes().ibc_compat()); let native_asset = genesis_state.native_asset_base_denomination(); state_tx.put_native_asset(native_asset); diff --git a/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_execute_transaction_with_every_action_snapshot.snap b/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_execute_transaction_with_every_action_snapshot.snap index 887dcec2b7..9be04fcf01 100644 --- a/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_execute_transaction_with_every_action_snapshot.snap +++ b/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_execute_transaction_with_every_action_snapshot.snap @@ -3,36 +3,36 @@ source: crates/astria-sequencer/src/app/tests_breaking_changes.rs expression: app.app_hash.as_bytes() --- [ - 241, - 61, - 69, - 211, - 65, - 119, - 7, - 14, - 200, - 183, - 75, - 232, - 68, - 126, - 56, - 5, - 8, - 220, - 199, - 173, - 193, - 135, - 169, - 26, - 36, - 119, - 144, - 174, + 205, + 122, + 180, + 182, + 185, + 118, + 201, + 16, + 156, + 198, + 47, + 115, + 196, + 240, + 1, 76, - 108, - 221, - 225 + 134, + 9, + 155, + 186, + 141, + 69, + 106, + 166, + 104, + 180, + 58, + 67, + 40, + 249, + 230, + 61 ] diff --git a/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_finalize_block_snapshot.snap b/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_finalize_block_snapshot.snap index 4647bcb04f..f254b8eade 100644 --- a/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_finalize_block_snapshot.snap +++ b/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_finalize_block_snapshot.snap @@ -3,36 +3,36 @@ source: crates/astria-sequencer/src/app/tests_breaking_changes.rs expression: app.app_hash.as_bytes() --- [ - 36, - 66, - 30, - 16, - 122, - 103, - 62, + 155, + 49, + 194, 67, - 145, - 180, - 247, - 139, - 229, - 200, - 86, - 193, - 28, - 71, - 235, - 32, - 134, - 143, - 239, - 229, - 8, - 86, - 190, - 164, + 213, + 224, + 18, + 135, + 75, + 34, + 48, + 234, + 42, + 79, + 85, + 103, 81, - 230, - 24, - 149 + 248, + 170, + 79, + 53, + 192, + 150, + 62, + 168, + 147, + 50, + 160, + 183, + 118, + 165, + 140 ] diff --git a/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_genesis_snapshot.snap b/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_genesis_snapshot.snap index 8d4956d126..7efbc10958 100644 --- a/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_genesis_snapshot.snap +++ b/crates/astria-sequencer/src/app/snapshots/astria_sequencer__app__tests_breaking_changes__app_genesis_snapshot.snap @@ -3,36 +3,36 @@ source: crates/astria-sequencer/src/app/tests_breaking_changes.rs expression: app.app_hash.as_bytes() --- [ - 107, - 187, - 174, - 122, - 174, - 31, - 236, - 104, - 223, - 121, - 241, - 170, - 212, + 65, 166, + 254, + 203, + 116, + 175, + 98, + 35, + 147, + 12, 199, - 70, - 87, - 126, - 205, - 179, - 18, - 34, - 54, - 131, - 22, - 122, + 221, + 27, + 189, + 119, 240, - 109, - 24, - 15, - 129, - 135 + 69, + 16, + 76, + 40, + 40, + 52, + 64, + 1, + 8, + 120, + 77, + 193, + 79, + 217, + 77, + 120 ] diff --git a/crates/astria-sequencer/src/app/test_utils.rs b/crates/astria-sequencer/src/app/test_utils.rs index 63bbaf885b..3acd71ed3c 100644 --- a/crates/astria-sequencer/src/app/test_utils.rs +++ b/crates/astria-sequencer/src/app/test_utils.rs @@ -90,9 +90,13 @@ pub(crate) fn default_fees() -> astria_core::protocol::genesis::v1alpha1::Fees { } pub(crate) fn address_prefixes() -> AddressPrefixes { - AddressPrefixes { - base: crate::test_utils::ASTRIA_PREFIX.into(), - } + AddressPrefixes::try_from_raw( + astria_core::generated::protocol::genesis::v1alpha1::AddressPrefixes { + base: crate::test_utils::ASTRIA_PREFIX.into(), + ibc_compat: crate::test_utils::ASTRIA_COMPAT_PREFIX.into(), + }, + ) + .unwrap() } pub(crate) fn proto_genesis_state() diff --git a/crates/astria-sequencer/src/authority/state_ext.rs b/crates/astria-sequencer/src/authority/state_ext.rs index f98d18a78f..2219b4ca49 100644 --- a/crates/astria-sequencer/src/authority/state_ext.rs +++ b/crates/astria-sequencer/src/authority/state_ext.rs @@ -150,7 +150,7 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + state.put_base_prefix(ASTRIA_PREFIX); // doesn't exist at first state diff --git a/crates/astria-sequencer/src/bridge/bridge_lock_action.rs b/crates/astria-sequencer/src/bridge/bridge_lock_action.rs index b2375aae24..7c9afa822c 100644 --- a/crates/astria-sequencer/src/bridge/bridge_lock_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_lock_action.rs @@ -188,7 +188,7 @@ mod tests { state.put_current_source(TransactionContext { address_bytes: from_address.bytes(), }); - state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + state.put_base_prefix(ASTRIA_PREFIX); state.put_transfer_base_fee(transfer_fee).unwrap(); state.put_bridge_lock_byte_cost_multiplier(2); diff --git a/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs b/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs index 1ebe568d53..2dfd2cd16b 100644 --- a/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_sudo_change_action.rs @@ -131,7 +131,7 @@ mod tests { state.put_current_source(TransactionContext { address_bytes: [1; 20], }); - state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + state.put_base_prefix(ASTRIA_PREFIX); let asset = test_asset(); state.put_allowed_fee_asset(&asset); @@ -167,7 +167,7 @@ mod tests { state.put_current_source(TransactionContext { address_bytes: sudo_address.bytes(), }); - state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + state.put_base_prefix(ASTRIA_PREFIX); state.put_bridge_sudo_change_base_fee(10); let fee_asset = test_asset(); diff --git a/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs b/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs index 615f77b9e2..88bab91a13 100644 --- a/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs @@ -141,7 +141,7 @@ mod tests { state.put_current_source(TransactionContext { address_bytes: [1; 20], }); - state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + state.put_base_prefix(ASTRIA_PREFIX); let asset = test_asset(); let transfer_amount = 100; @@ -178,7 +178,7 @@ mod tests { state.put_current_source(TransactionContext { address_bytes: [1; 20], }); - state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + state.put_base_prefix(ASTRIA_PREFIX); let asset = test_asset(); let transfer_amount = 100; @@ -208,62 +208,6 @@ mod tests { ); } - #[tokio::test] - async fn execute_with_bridge_address_set() { - let storage = cnidarium::TempStorage::new().await.unwrap(); - let snapshot = storage.latest_snapshot(); - let mut state = StateDelta::new(snapshot); - - let bridge_address = astria_address(&[1; 20]); - state.put_current_source(TransactionContext { - address_bytes: bridge_address.bytes(), - }); - state.put_base_prefix(ASTRIA_PREFIX).unwrap(); - - let asset = test_asset(); - let transfer_fee = 10; - let transfer_amount = 100; - state.put_transfer_base_fee(transfer_fee).unwrap(); - - let to_address = astria_address(&[2; 20]); - let rollup_id = RollupId::from_unhashed_bytes(b"test_rollup_id"); - - state.put_bridge_account_rollup_id(bridge_address, &rollup_id); - state - .put_bridge_account_ibc_asset(bridge_address, &asset) - .unwrap(); - state.put_bridge_account_withdrawer_address(bridge_address, bridge_address); - state.put_allowed_fee_asset(&asset); - - let bridge_unlock = BridgeUnlockAction { - to: to_address, - amount: transfer_amount, - fee_asset: asset.clone(), - memo: String::new(), - bridge_address, - rollup_block_number: 1, - rollup_withdrawal_event_id: "a-rollup-defined-hash-3".to_string(), - }; - - // not enough balance; should fail - state - .put_account_balance(bridge_address, &asset, transfer_amount) - .unwrap(); - assert_anyhow_error( - &bridge_unlock - .check_and_execute(&mut state) - .await - .unwrap_err(), - "insufficient funds for transfer and fee payment", - ); - - // enough balance; should pass - state - .put_account_balance(bridge_address, &asset, transfer_amount + transfer_fee) - .unwrap(); - bridge_unlock.check_and_execute(&mut state).await.unwrap(); - } - #[tokio::test] async fn execute_with_duplicated_withdrawal_event_id() { let storage = cnidarium::TempStorage::new().await.unwrap(); @@ -274,7 +218,7 @@ mod tests { state.put_current_source(TransactionContext { address_bytes: bridge_address.bytes(), }); - state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + state.put_base_prefix(ASTRIA_PREFIX); let asset = test_asset(); let transfer_fee = 10; diff --git a/crates/astria-sequencer/src/bridge/query.rs b/crates/astria-sequencer/src/bridge/query.rs index 7807ebb188..9b7050f8ce 100644 --- a/crates/astria-sequencer/src/bridge/query.rs +++ b/crates/astria-sequencer/src/bridge/query.rs @@ -314,7 +314,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + state.put_base_prefix(ASTRIA_PREFIX); let asset: astria_core::primitive::v1::asset::Denom = "test".parse().unwrap(); let rollup_id = RollupId::from_unhashed_bytes("test"); diff --git a/crates/astria-sequencer/src/ibc/ics20_transfer.rs b/crates/astria-sequencer/src/ibc/ics20_transfer.rs index 9149859455..7372d284e5 100644 --- a/crates/astria-sequencer/src/ibc/ics20_transfer.rs +++ b/crates/astria-sequencer/src/ibc/ics20_transfer.rs @@ -24,6 +24,8 @@ use astria_core::{ Denom, }, Address, + Bech32, + Bech32m, }, protocol::memos, sequencerblock::v1alpha1::block::Deposit, @@ -57,9 +59,12 @@ use penumbra_ibc::component::app_handler::{ AppHandlerExecute, }; use penumbra_proto::penumbra::core::component::ibc::v1::FungibleTokenPacketData; +use tokio::try_join; +use tracing::instrument; use crate::{ accounts::StateWriteExt as _, + address::StateReadExt as _, assets::{ StateReadExt as _, StateWriteExt as _, @@ -376,16 +381,27 @@ async fn execute_ics20_transfer( ) -> Result<()> { let packet_data: FungibleTokenPacketData = serde_json::from_slice(data).context("failed to decode FungibleTokenPacketData")?; + + // if the memo deserializes into an `Ics20WithdrawalFromRollupMemo`, + // we can assume this is a refund from an attempted withdrawal from + // a rollup directly to another IBC chain via the sequencer. + // + // in this case, we lock the tokens back in the bridge account and + // emit a `Deposit` event to send the tokens back to the rollup. + if is_refund + && serde_json::from_str::(&packet_data.memo) + .is_ok() + { + execute_withdrawal_refund_to_rollup(state, packet_data) + .await + .context("failed to execute rollup withdrawal refund")?; + return Ok(()); + } + let packet_amount: u128 = packet_data .amount .parse() .context("failed to parse packet data amount to u128")?; - let recipient = if is_refund { - packet_data.sender.clone() - } else { - packet_data.receiver - }; - let mut denom_trace = { let denom = packet_data .denom @@ -398,33 +414,12 @@ async fn execute_ics20_transfer( .context("failed to convert denomination if ibc/ prefixed")? }; - // if the memo deserializes into an `Ics20WithdrawalFromRollupMemo`, - // we can assume this is a refund from an attempted withdrawal from - // a rollup directly to another IBC chain via the sequencer. - // - // in this case, we lock the tokens back in the bridge account and - // emit a `Deposit` event to send the tokens back to the rollup. - if is_refund - && serde_json::from_str::(&packet_data.memo) - .is_ok() - { - let bridge_account = packet_data.sender.parse().context( - "sender not an Astria Address: for refunds of ics20 withdrawals that came from a \ - rollup, the sender must be a valid Astria Address (usually the bridge account)", - )?; - execute_rollup_withdrawal_refund( - state, - bridge_account, - &denom_trace, - packet_amount, - recipient, - ) - .await - .context("failed to execute rollup withdrawal refund")?; - return Ok(()); - } - // the IBC packet should have the address as a bech32 string + let recipient = if is_refund { + packet_data.sender.clone() + } else { + packet_data.receiver + }; let recipient = recipient.parse().context("invalid recipient address")?; let is_prefixed = denom_trace.starts_with_str(&format!("{source_port}/{source_channel}")); @@ -513,31 +508,109 @@ async fn execute_ics20_transfer( Ok(()) } -/// execute a refund of tokens that were withdrawn from a rollup to another -/// IBC-enabled chain via the sequencer using an `Ics20Withdrawal`, but were not -/// transferred to the destination IBC chain successfully. +/// Execute a refund for a failed ics20 transfer from a rollup to a remote IBC chain. +/// +/// A withdrawal of tokens from a rollup to a remote IBC chain is started with a +/// `Ics20Withdrawal` action via a (rollup's) bridge account on the sequencer. +/// +/// This function then sends the tokens back to the rollup via a `Deposit` event, +/// and again locks the tokens in the specified bridge account. +/// +/// This function must only be called if the following conditions hold: +/// 1. The ics20 tranfer is a refund; +/// 2. The memo contained in the ics20 transfer packet can be parsed as a Ics20WithdrawalFromRollup. /// -/// this functions sends the tokens back to the rollup via a `Deposit` event, -/// and locks the tokens back in the specified bridge account. -async fn execute_rollup_withdrawal_refund( +/// The function then assumes that the `packet_data.sender` is the bridge account, and +/// attempts to parse it as either a base-prefixed bech32m address, or as an ibc-compat-prefixed +/// bech32 address. If the sender is an ibc-compat address, then it will be converted to its +/// base-prefixed version. +/// +/// The emitted deposit as seen by the rollup will *never* be the ibc-compat prefixed version. +// TODO: Add `err` with https://github.com/astriaorg/astria/issues/1386 being done +#[instrument(skip_all)] +async fn execute_withdrawal_refund_to_rollup( state: &mut S, - bridge_address: Address, - denom: &denom::TracePrefixed, - amount: u128, - destination_address: String, + packet_data: FungibleTokenPacketData, ) -> Result<()> { - execute_deposit(state, bridge_address, denom, amount, destination_address).await?; + let amount: u128 = packet_data + .amount + .parse() + .context("failed to parse packet data amount to u128")?; + let denom = { + let denom = packet_data + .denom + .parse::() + .context("failed parsing denom in packet data as Denom")?; + // convert denomination if it's prefixed with `ibc/` + // note: this denomination might have a prefix, but it wasn't prefixed by us right now. + convert_denomination_if_ibc_prefixed(state, denom) + .await + .context("failed to convert denomination if ibc/ prefixed")? + }; + let bridge_address = parse_refund_sender(&*state, &packet_data.sender) + .await + .context("failed to parse ibc packet sender as the refund target address")?; + execute_deposit( + state, + bridge_address, + &denom, + amount, + bridge_address.to_string(), + ) + .await + .context("failed to emit deposit")?; state .increase_balance(bridge_address, denom, amount) .await - .context( - "failed to update bridge account account balance in execute_rollup_withdrawal_refund", - )?; + .context("failed to update bridge account account balance")?; Ok(()) } +async fn parse_refund_sender(state: &S, sender: &str) -> anyhow::Result
{ + use futures::TryFutureExt as _; + let (base_prefix, compat_prefix) = match try_join!( + state + .get_base_prefix() + .map_err(|e| e.context("failed to read base prefix from state")), + state + .get_ibc_compat_prefix() + .map_err(|e| e.context("failed to read ibc compat prefix from state")) + ) { + Ok(prefixes) => prefixes, + Err(err) => return Err(err), + }; + sender + .parse::>() + .context("failed to parse address in bech32m format") + .and_then(|addr| { + ensure!( + addr.prefix() == base_prefix, + "address prefix is not base prefix stored in state" + ); + Ok(addr) + }) + .or_else(|_| { + sender + .parse::>() + .context("failed to parse address in bech32/compat format") + .and_then(|addr| { + ensure!( + addr.prefix() == compat_prefix, + "address prefix is not base prefix stored in state" + ); + addr.to_prefix(&base_prefix) + .context( + "failed to convert ibc compat prefixed address to standard base \ + prefixed address", + ) + .map(|addr| addr.to_format::()) + }) + }) + // "sender address was neither base nor ibc-compat prefixed; returning last error", +} + /// execute an ics20 transfer where the recipient is a bridge account. /// /// if the recipient is not a bridge account, or the incoming packet is a refund, @@ -649,10 +722,14 @@ mod test { use super::*; use crate::{ accounts::StateReadExt as _, + address::StateWriteExt, ibc::StateWriteExt as _, test_utils::{ astria_address, astria_address_from_hex_string, + astria_compat_address, + ASTRIA_COMPAT_PREFIX, + ASTRIA_PREFIX, }, }; @@ -991,7 +1068,11 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot.clone()); + state_tx.put_base_prefix(ASTRIA_PREFIX); + state_tx.put_ibc_compat_prefix(ASTRIA_COMPAT_PREFIX); + let bridge_address = astria_address(&[99u8; 20]); + let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); let denom = "dest_port/dest_channel/nootasset" .parse::() @@ -1003,16 +1084,18 @@ mod test { .unwrap(); let amount = 100; - let destination_address = "destinationaddress".to_string(); - execute_rollup_withdrawal_refund( - &mut state_tx, - bridge_address, - &denom, - amount, - destination_address, - ) - .await - .expect("valid rollup withdrawal refund"); + let address_on_rollup = "address_on_rollup".to_string(); + + let packet = FungibleTokenPacketData { + denom: denom.to_string(), + sender: bridge_address.to_string(), + amount: amount.to_string(), + receiver: address_on_rollup.to_string(), + memo: String::new(), + }; + execute_withdrawal_refund_to_rollup(&mut state_tx, packet) + .await + .expect("valid rollup withdrawal refund"); let balance = state_tx .get_account_balance(bridge_address, denom) @@ -1028,11 +1111,14 @@ mod test { } #[tokio::test] - async fn execute_ics20_transfer_rollup_withdrawal_refund() { + async fn rollup_withdrawal_refund_succeeds() { let storage = cnidarium::TempStorage::new().await.unwrap(); let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot.clone()); + state_tx.put_base_prefix(ASTRIA_PREFIX); + state_tx.put_ibc_compat_prefix(ASTRIA_COMPAT_PREFIX); + let bridge_address = astria_address(&[99u8; 20]); let destination_chain_address = bridge_address.to_string(); let denom = "nootasset".parse::().unwrap(); @@ -1095,4 +1181,79 @@ mod test { ); assert_eq!(deposit, &expected_deposit); } + + #[tokio::test] + async fn rollup_withdrawal_refund_with_compat_address_succeeds() { + let storage = cnidarium::TempStorage::new().await.unwrap(); + let snapshot = storage.latest_snapshot(); + let mut state_tx = StateDelta::new(snapshot.clone()); + + state_tx.put_base_prefix(ASTRIA_PREFIX); + state_tx.put_ibc_compat_prefix(ASTRIA_COMPAT_PREFIX); + + let bridge_address = astria_address(&[99u8; 20]); + let bridge_address_compat = astria_compat_address(&[99u8; 20]); + let denom = "nootasset".parse::().unwrap(); + let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); + + state_tx.put_bridge_account_rollup_id(bridge_address, &rollup_id); + state_tx + .put_bridge_account_ibc_asset(bridge_address, &denom) + .unwrap(); + + let packet = FungibleTokenPacketData { + denom: denom.to_string(), + sender: bridge_address_compat.to_string(), + amount: "100".to_string(), + receiver: "other-chain-address".to_string(), + memo: serde_json::to_string(&memos::v1alpha1::Ics20WithdrawalFromRollup { + memo: String::new(), + rollup_block_number: 1, + rollup_return_address: "rollup-defined".to_string(), + rollup_withdrawal_event_id: hex::encode([1u8; 32]), + }) + .unwrap(), + }; + let packet_bytes = serde_json::to_vec(&packet).unwrap(); + + execute_ics20_transfer( + &mut state_tx, + &packet_bytes, + &"source_port".to_string().parse().unwrap(), + &"source_channel".to_string().parse().unwrap(), + &"source_port".to_string().parse().unwrap(), + &"source_channel".to_string().parse().unwrap(), + true, + ) + .await + .expect("valid ics20 transfer refund; recipient, memo, and asset ID are valid"); + + let balance = state_tx + .get_account_balance(bridge_address, &denom) + .await + .expect( + "ics20 transfer refunding to rollup should succeed and balance should be added to \ + the bridge account", + ); + assert_eq!(balance, 100); + + let deposits = state_tx + .get_block_deposits() + .await + .expect("a deposit should exist as a result of the rollup withdrawal refund"); + assert_eq!(deposits.len(), 1); + + let deposit = deposits.get(&rollup_id).unwrap().first().unwrap(); + let expected_deposit = Deposit::new( + bridge_address, + rollup_id, + 100, + denom, + bridge_address.to_string(), /* NOTE: this is the non-compat address because it will + * be converted from the compat bech32 to the + * standard/non-compat bech32m version before emitting + * the deposit event */ + ); + assert_eq!(deposit, &expected_deposit); + } } diff --git a/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs b/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs index 515befa6ba..ee91e52eee 100644 --- a/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs +++ b/crates/astria-sequencer/src/ibc/ics20_withdrawal.rs @@ -8,6 +8,7 @@ use astria_core::{ primitive::v1::{ asset::Denom, Address, + Bech32, }, protocol::{ memos::v1alpha1::Ics20WithdrawalFromRollup, @@ -29,6 +30,7 @@ use penumbra_ibc::component::packet::{ SendPacketWrite as _, Unchecked, }; +use penumbra_proto::core::component::ibc::v1::FungibleTokenPacketData; use crate::{ accounts::{ @@ -50,20 +52,42 @@ use crate::{ transaction::StateReadExt as _, }; -fn withdrawal_to_unchecked_ibc_packet( +async fn create_ibc_packet_from_withdrawal( withdrawal: &action::Ics20Withdrawal, -) -> IBCPacket { - let packet_data = withdrawal.to_fungible_token_packet_data(); + state: S, +) -> anyhow::Result> { + let sender = if withdrawal.use_compat_address { + let ibc_compat_prefix = state.get_ibc_compat_prefix().await.context( + "need to construct bech32 compatible address for IBC communication but failed reading \ + required prefix from state", + )?; + withdrawal + .return_address() + .to_prefix(&ibc_compat_prefix) + .context("failed to convert the address to the bech32 compatible prefix")? + .to_format::() + .to_string() + } else { + withdrawal.return_address.to_string() + }; + let packet = FungibleTokenPacketData { + amount: withdrawal.amount.to_string(), + denom: withdrawal.denom.to_string(), + sender, + receiver: withdrawal.destination_chain_address.clone(), + memo: withdrawal.memo.clone(), + }; + let serialized_packet_data = - serde_json::to_vec(&packet_data).expect("can serialize FungibleTokenPacketData as JSON"); + serde_json::to_vec(&packet).context("failed to serialize fungible token packet as JSON")?; - IBCPacket::new( + Ok(IBCPacket::new( PortId::transfer(), withdrawal.source_channel().clone(), *withdrawal.timeout_height(), withdrawal.timeout_time(), serialized_packet_data, - ) + )) } /// Establishes the withdrawal target. @@ -193,7 +217,9 @@ impl ActionHandler for action::Ics20Withdrawal { .await .context("failed to get block timestamp")?; let packet = { - let packet = withdrawal_to_unchecked_ibc_packet(self); + let packet = create_ibc_packet_from_withdrawal(self, &state) + .await + .context("failed converting the withdrawal action into IBC packet")?; state .send_packet_check(packet, current_timestamp) .await @@ -282,6 +308,7 @@ mod tests { source_channel: "channel-0".to_string().parse().unwrap(), fee_asset: denom.clone(), memo: String::new(), + use_compat_address: false, }; assert_eq!( @@ -298,7 +325,7 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + state.put_base_prefix(ASTRIA_PREFIX); // sender is a bridge address, which is also the withdrawer, so it's ok let bridge_address = [1u8; 20]; @@ -320,6 +347,7 @@ mod tests { source_channel: "channel-0".to_string().parse().unwrap(), fee_asset: denom.clone(), memo: String::new(), + use_compat_address: false, }; assert_anyhow_error( @@ -353,6 +381,7 @@ mod tests { source_channel: "channel-0".to_string().parse().unwrap(), fee_asset: denom(), memo: String::new(), + use_compat_address: false, } } @@ -361,7 +390,7 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + state.put_base_prefix(ASTRIA_PREFIX); // withdraw is *not* the bridge address, Ics20Withdrawal must be sent by the withdrawer state.put_bridge_account_rollup_id( @@ -395,7 +424,7 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + state.put_base_prefix(ASTRIA_PREFIX); // sender the withdrawer address, so it's ok let bridge_address = [1u8; 20]; @@ -418,6 +447,7 @@ mod tests { source_channel: "channel-0".to_string().parse().unwrap(), fee_asset: denom.clone(), memo: String::new(), + use_compat_address: false, }; assert_eq!( @@ -449,6 +479,7 @@ mod tests { source_channel: "channel-0".to_string().parse().unwrap(), fee_asset: denom.clone(), memo: String::new(), + use_compat_address: false, }; assert_anyhow_error( diff --git a/crates/astria-sequencer/src/ibc/state_ext.rs b/crates/astria-sequencer/src/ibc/state_ext.rs index 880f86f6ba..b5c230bf98 100644 --- a/crates/astria-sequencer/src/ibc/state_ext.rs +++ b/crates/astria-sequencer/src/ibc/state_ext.rs @@ -227,7 +227,7 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + state.put_base_prefix(ASTRIA_PREFIX); // can write new let mut address = [42u8; 20]; @@ -264,7 +264,7 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + state.put_base_prefix(ASTRIA_PREFIX); // unset address returns false let address = astria_address(&[42u8; 20]); @@ -283,7 +283,7 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + state.put_base_prefix(ASTRIA_PREFIX); // can write let address = astria_address(&[42u8; 20]); @@ -313,7 +313,7 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - state.put_base_prefix(ASTRIA_PREFIX).unwrap(); + state.put_base_prefix(ASTRIA_PREFIX); // can write let address = astria_address(&[42u8; 20]); diff --git a/crates/astria-sequencer/src/service/info/mod.rs b/crates/astria-sequencer/src/service/info/mod.rs index c0135c13ea..11e1348036 100644 --- a/crates/astria-sequencer/src/service/info/mod.rs +++ b/crates/astria-sequencer/src/service/info/mod.rs @@ -216,7 +216,7 @@ mod test { let mut state = StateDelta::new(storage.latest_snapshot()); state.put_storage_version_by_height(height, version); - state.put_base_prefix("astria").unwrap(); + state.put_base_prefix("astria"); state.put_native_asset(&crate::test_utils::nria()); let address = state diff --git a/crates/astria-sequencer/src/test_utils.rs b/crates/astria-sequencer/src/test_utils.rs index 7684f8f02f..f39ee12d71 100644 --- a/crates/astria-sequencer/src/test_utils.rs +++ b/crates/astria-sequencer/src/test_utils.rs @@ -1,9 +1,11 @@ use astria_core::primitive::v1::{ asset::TracePrefixed, Address, + Bech32, }; pub(crate) const ASTRIA_PREFIX: &str = "astria"; +pub(crate) const ASTRIA_COMPAT_PREFIX: &str = "astriacompat"; pub(crate) fn astria_address(bytes: &[u8]) -> Address { Address::builder() @@ -13,6 +15,15 @@ pub(crate) fn astria_address(bytes: &[u8]) -> Address { .unwrap() } +#[cfg_attr(feature = "benchmark", allow(dead_code))] +pub(crate) fn astria_compat_address(bytes: &[u8]) -> Address { + Address::builder() + .prefix(ASTRIA_COMPAT_PREFIX) + .slice(bytes) + .try_build() + .unwrap() +} + pub(crate) fn astria_address_from_hex_string(s: &str) -> Address { let bytes = hex::decode(s).unwrap(); Address::builder() diff --git a/crates/astria-sequencer/src/transaction/checks.rs b/crates/astria-sequencer/src/transaction/checks.rs index 77aad88236..47e3986bf9 100644 --- a/crates/astria-sequencer/src/transaction/checks.rs +++ b/crates/astria-sequencer/src/transaction/checks.rs @@ -324,6 +324,7 @@ mod tests { bridge::StateWriteExt as _, ibc::StateWriteExt as _, sequence::StateWriteExt as _, + test_utils::ASTRIA_PREFIX, }; #[tokio::test] @@ -332,7 +333,7 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot); - state_tx.put_base_prefix("astria").unwrap(); + state_tx.put_base_prefix("astria"); state_tx.put_native_asset(&crate::test_utils::nria()); state_tx.put_transfer_base_fee(12).unwrap(); state_tx.put_sequence_action_base_fee(0); @@ -409,7 +410,7 @@ mod tests { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot); - state_tx.put_base_prefix("nria").unwrap(); + state_tx.put_base_prefix(ASTRIA_PREFIX); state_tx.put_native_asset(&crate::test_utils::nria()); state_tx.put_transfer_base_fee(12).unwrap(); state_tx.put_sequence_action_base_fee(0); diff --git a/proto/protocolapis/astria/protocol/genesis/v1alpha1/types.proto b/proto/protocolapis/astria/protocol/genesis/v1alpha1/types.proto index 89fa8fb3ae..caf199db59 100644 --- a/proto/protocolapis/astria/protocol/genesis/v1alpha1/types.proto +++ b/proto/protocolapis/astria/protocol/genesis/v1alpha1/types.proto @@ -23,7 +23,11 @@ message Account { } message AddressPrefixes { + // The base prefix used for most Astria Sequencer addresses. string base = 1; + // The prefix used for sending ics20 transfers to IBC chains + // that enforce a bech32 format of the packet sender. + string ibc_compat = 2; } // IBC configuration data. diff --git a/proto/protocolapis/astria/protocol/transactions/v1alpha1/types.proto b/proto/protocolapis/astria/protocol/transactions/v1alpha1/types.proto index 05f40dc21b..46342da5ef 100644 --- a/proto/protocolapis/astria/protocol/transactions/v1alpha1/types.proto +++ b/proto/protocolapis/astria/protocol/transactions/v1alpha1/types.proto @@ -130,6 +130,11 @@ message Ics20Withdrawal { // if unset, and the transaction sender is a bridge account, the withdrawal is // treated as a bridge withdrawal (ie. the bridge account's withdrawer address is checked). astria.primitive.v1.Address bridge_address = 10; + + // whether to use a bech32-compatible format of the `.return_address` when generating + // fungible token packets (as opposed to Astria-native bech32m addresses). This is + // necessary for chains like noble which enforce a strict bech32 format. + bool use_compat_address = 11; } message IbcHeight {