From 13aae9cda4aaa37c0f5e4c830828158d38922bb0 Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Thu, 30 May 2024 16:46:48 +0200 Subject: [PATCH 01/15] feat(core, proto): add bech32m addresses --- Cargo.lock | 1 + crates/astria-core/Cargo.toml | 1 + .../src/generated/astria.primitive.v1.rs | 5 +- .../generated/astria.primitive.v1.serde.rs | 41 +++- crates/astria-core/src/primitive/v1/mod.rs | 210 +++++++++++++++--- ...core__primitive__v1__tests__snapshots.snap | 5 +- .../protocol/transaction/v1alpha1/action.rs | 78 ++++--- ...alpha1__test__signed_transaction_hash.snap | 58 ++--- .../src/sequencerblock/v1alpha1/block.rs | 16 +- .../astria/primitive/v1/types.proto | 4 +- 10 files changed, 310 insertions(+), 109 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 84c3eb3c4f..6e0fa7bb50 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -638,6 +638,7 @@ dependencies = [ "astria-merkle", "base64 0.21.7", "base64-serde", + "bech32 0.11.0", "brotli", "bytes", "celestia-tendermint", diff --git a/crates/astria-core/Cargo.toml b/crates/astria-core/Cargo.toml index 7d273b985e..9d2c203915 100644 --- a/crates/astria-core/Cargo.toml +++ b/crates/astria-core/Cargo.toml @@ -44,6 +44,7 @@ tracing = { workspace = true } base64-serde = { workspace = true, optional = true } base64 = { workspace = true } zeroize = { version = "1.7.0", features = ["zeroize_derive"] } +bech32 = "0.11.0" [features] celestia = ["dep:celestia-types"] diff --git a/crates/astria-core/src/generated/astria.primitive.v1.rs b/crates/astria-core/src/generated/astria.primitive.v1.rs index eab0146c7c..31d7057a35 100644 --- a/crates/astria-core/src/generated/astria.primitive.v1.rs +++ b/crates/astria-core/src/generated/astria.primitive.v1.rs @@ -86,8 +86,11 @@ impl ::prost::Name for RollupId { #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct Address { + #[deprecated] #[prost(bytes = "bytes", tag = "1")] - pub inner: ::prost::bytes::Bytes, + pub bytes: ::prost::bytes::Bytes, + #[prost(string, tag = "2")] + pub bech32m: ::prost::alloc::string::String, } impl ::prost::Name for Address { const NAME: &'static str = "Address"; diff --git a/crates/astria-core/src/generated/astria.primitive.v1.serde.rs b/crates/astria-core/src/generated/astria.primitive.v1.serde.rs index ffa9858f43..90f3ca4ed4 100644 --- a/crates/astria-core/src/generated/astria.primitive.v1.serde.rs +++ b/crates/astria-core/src/generated/astria.primitive.v1.serde.rs @@ -6,13 +6,19 @@ impl serde::Serialize for Address { { use serde::ser::SerializeStruct; let mut len = 0; - if !self.inner.is_empty() { + if !self.bytes.is_empty() { + len += 1; + } + if !self.bech32m.is_empty() { len += 1; } let mut struct_ser = serializer.serialize_struct("astria.primitive.v1.Address", len)?; - if !self.inner.is_empty() { + if !self.bytes.is_empty() { #[allow(clippy::needless_borrow)] - struct_ser.serialize_field("inner", pbjson::private::base64::encode(&self.inner).as_str())?; + struct_ser.serialize_field("bytes", pbjson::private::base64::encode(&self.bytes).as_str())?; + } + if !self.bech32m.is_empty() { + struct_ser.serialize_field("bech32m", &self.bech32m)?; } struct_ser.end() } @@ -24,12 +30,14 @@ impl<'de> serde::Deserialize<'de> for Address { D: serde::Deserializer<'de>, { const FIELDS: &[&str] = &[ - "inner", + "bytes", + "bech32m", ]; #[allow(clippy::enum_variant_names)] enum GeneratedField { - Inner, + Bytes, + Bech32m, } impl<'de> serde::Deserialize<'de> for GeneratedField { fn deserialize(deserializer: D) -> std::result::Result @@ -51,7 +59,8 @@ impl<'de> serde::Deserialize<'de> for Address { E: serde::de::Error, { match value { - "inner" => Ok(GeneratedField::Inner), + "bytes" => Ok(GeneratedField::Bytes), + "bech32m" => Ok(GeneratedField::Bech32m), _ => Err(serde::de::Error::unknown_field(value, FIELDS)), } } @@ -71,21 +80,29 @@ impl<'de> serde::Deserialize<'de> for Address { where V: serde::de::MapAccess<'de>, { - let mut inner__ = None; + let mut bytes__ = None; + let mut bech32m__ = None; while let Some(k) = map_.next_key()? { match k { - GeneratedField::Inner => { - if inner__.is_some() { - return Err(serde::de::Error::duplicate_field("inner")); + GeneratedField::Bytes => { + if bytes__.is_some() { + return Err(serde::de::Error::duplicate_field("bytes")); } - inner__ = + bytes__ = Some(map_.next_value::<::pbjson::private::BytesDeserialize<_>>()?.0) ; } + GeneratedField::Bech32m => { + if bech32m__.is_some() { + return Err(serde::de::Error::duplicate_field("bech32m")); + } + bech32m__ = Some(map_.next_value()?); + } } } Ok(Address { - inner: inner__.unwrap_or_default(), + bytes: bytes__.unwrap_or_default(), + bech32m: bech32m__.unwrap_or_default(), }) } } diff --git a/crates/astria-core/src/primitive/v1/mod.rs b/crates/astria-core/src/primitive/v1/mod.rs index 944d6e40bd..e3c329200a 100644 --- a/crates/astria-core/src/primitive/v1/mod.rs +++ b/crates/astria-core/src/primitive/v1/mod.rs @@ -3,7 +3,10 @@ pub mod u128; use base64::{ display::Base64Display, - prelude::BASE64_STANDARD, + prelude::{ + Engine as _, + BASE64_STANDARD, + }, }; use sha2::{ Digest as _, @@ -16,6 +19,14 @@ use crate::{ }; pub const ADDRESS_LEN: usize = 20; +/// The human readable prefix of astria addresses (also known as bech32 HRP). +pub const HUMAN_READABLE_ADDRESS_PREFIX: &str = "astria"; +// The compile-time generated bech32::Hrp to avoid redoing it on every encode. +// Intentionally kept crate-private to not make bech32 part of the crate API. +const BECH32_HRP: bech32::Hrp = bech32::Hrp::parse_unchecked(HUMAN_READABLE_ADDRESS_PREFIX); +// The length of astria addresses as bech32m strings. +const ADDRESS_BECH32M_LENGTH: usize = 45; + pub const ROLLUP_ID_LEN: usize = 32; pub const FEE_ASSET_ID_LEN: usize = 32; @@ -243,19 +254,60 @@ pub struct IncorrectRollupIdLength { received: usize, } -/// Indicates that the protobuf response contained an array field that was not 20 bytes long. #[derive(Debug, thiserror::Error)] -#[error("expected 20 bytes, got {received}")] -pub struct IncorrectAddressLength { - received: usize, +#[error(transparent)] +pub struct AddressError(AddressErrorKind); + +impl AddressError { + fn bech32m_decode(source: bech32::DecodeError) -> Self { + Self(AddressErrorKind::Bech32mDecode { + source, + }) + } + + fn fields_dont_match(bytes: [u8; ADDRESS_LEN], bech32m: [u8; ADDRESS_LEN]) -> Self { + Self(AddressErrorKind::FieldsDontMatch { + bytes, + bech32m, + }) + } + + fn incorrect_address_length(received: usize) -> Self { + Self(AddressErrorKind::IncorrectAddressLength { + received, + }) + } + + fn unknown_bech32_hrp(received: bech32::Hrp) -> Self { + Self(AddressErrorKind::UnknownBech32Hrp { + received, + }) + } +} + +#[derive(Debug, thiserror::Error, PartialEq)] +enum AddressErrorKind { + #[error("failed decoding provided bech32m string")] + Bech32mDecode { source: bech32::DecodeError }, + #[error( + "address protobuf contained mismatched address in its `bytes` ({}) and `bech32m` ({}) fields", + BASE64_STANDARD.encode(.bytes), + BASE64_STANDARD.encode(.bech32m), + )] + FieldsDontMatch { + bytes: [u8; ADDRESS_LEN], + bech32m: [u8; ADDRESS_LEN], + }, + #[error("expected an address of 20 bytes, got `{received}`")] + IncorrectAddressLength { received: usize }, + #[error("expected `\"astria\"` as the bech32 human readable prefix, got `\"{received}\"`")] + UnknownBech32Hrp { received: bech32::Hrp }, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] #[cfg_attr(feature = "serde", derive(serde::Serialize))] -pub struct Address( - #[cfg_attr(feature = "serde", serde(serialize_with = "crate::serde::base64"))] - [u8; ADDRESS_LEN], -); +#[cfg_attr(feature = "serde", serde(into = "raw::Address"))] +pub struct Address([u8; ADDRESS_LEN]); impl Address { #[must_use] @@ -273,30 +325,55 @@ impl Address { /// # Errors /// /// Returns an error if the account buffer was not 20 bytes long. - pub fn try_from_slice(bytes: &[u8]) -> Result { - let inner = <[u8; ADDRESS_LEN]>::try_from(bytes).map_err(|_| IncorrectAddressLength { - received: bytes.len(), - })?; + pub fn try_from_slice(bytes: &[u8]) -> Result { + let inner = <[u8; ADDRESS_LEN]>::try_from(bytes) + .map_err(|_| AddressError::incorrect_address_length(bytes.len()))?; Ok(Self::from_array(inner)) } + /// Convert a string containing a bech32m string to an astria address. + /// + /// # Errors + /// Returns an error if: + /// + `input` is not bech32m encoded. + /// + the human readable prefix (bech32 HRP) is not `"astria"`. + /// + the decoded data contained in `input` is not 20 bytes long. + pub fn try_from_bech32m(input: &str) -> Result { + let (hrp, bytes) = bech32::decode(input).map_err(AddressError::bech32m_decode)?; + if hrp != BECH32_HRP { + return Err(AddressError::unknown_bech32_hrp(hrp)); + } + Self::try_from_slice(&bytes) + } + #[must_use] pub const fn from_array(array: [u8; ADDRESS_LEN]) -> Self { Self(array) } + /// 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 mut bech32m = String::with_capacity(ADDRESS_BECH32M_LENGTH); + bech32::encode_lower_to_fmt::(&mut bech32m, BECH32_HRP, &self.get()) + .expect( + "must not fail because Address is tested to be ADDRESS_BECH32M_LENGTH long, which \ + is less than the permitted maximum bech32m checksum length", + ); + // allow: for compatibility purposes. The `bytes` protobuf field is deprecated + // and should not be used by downstream users in new code. + #[allow(deprecated)] raw::Address { - inner: self.to_vec().into(), + bytes: self.to_vec().into(), + bech32m, } } #[must_use] pub fn into_raw(self) -> raw::Address { - raw::Address { - inner: self.to_vec().into(), - } + self.to_raw() } /// Convert from protobuf to rust type an address. @@ -304,8 +381,29 @@ impl Address { /// # Errors /// /// Returns an error if the account buffer was not 20 bytes long. - pub fn try_from_raw(raw: &raw::Address) -> Result { - Self::try_from_slice(&raw.inner) + pub fn try_from_raw(raw: &raw::Address) -> Result { + // allow: for compatibility purposes. The `bytes` protobuf field is deprecated + // and should not be used by downstream users in new code. + #![allow(deprecated)] + let raw::Address { + bytes, + bech32m, + } = raw; + if bech32m.is_empty() { + return Self::try_from_slice(bytes); + } + if bytes.is_empty() { + return Self::try_from_bech32m(bech32m); + } + let addr_bytes = Self::try_from_slice(bytes)?; + let addr_bech32m = Self::try_from_bech32m(bech32m)?; + if addr_bytes != addr_bech32m { + return Err(AddressError::fields_dont_match( + addr_bytes.get(), + addr_bech32m.get(), + )); + } + Ok(addr_bytes) } } @@ -321,9 +419,24 @@ impl From<[u8; ADDRESS_LEN]> for Address { } } +impl From
for raw::Address { + fn from(value: Address) -> Self { + value.into_raw() + } +} + impl std::fmt::Display for Address { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - Base64Display::new(self.as_ref(), &BASE64_STANDARD).fmt(f) + use bech32::EncodeError; + match bech32::encode_lower_to_fmt::(f, BECH32_HRP, &self.get()) { + 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:?}", + ), + } } } @@ -347,11 +460,13 @@ where #[cfg(test)] mod tests { + use bytes::Bytes; use insta::assert_json_snapshot; use super::{ Address, - IncorrectAddressLength, + AddressErrorKind, + ADDRESS_LEN, }; #[test] @@ -364,10 +479,8 @@ mod tests { #[track_caller] fn account_conversion_check(bad_account: &[u8]) { - let error = Address::try_from_slice(bad_account); - assert!( - matches!(error, Err(IncorrectAddressLength { .. })), - "converting form incorrect sized account succeeded where it should have failed" + Address::try_from_slice(bad_account).expect_err( + "converting from an incorrectly sized byte slice succeeded where it should have failed", ); } @@ -384,4 +497,51 @@ mod tests { let address = Address([42; 20]); assert_json_snapshot!(address); } + + #[test] + fn const_astria_hrp_is_valid() { + let hrp = bech32::Hrp::parse(super::HUMAN_READABLE_ADDRESS_PREFIX).unwrap(); + assert_eq!(hrp, super::BECH32_HRP); + } + + #[test] + fn const_astria_bech32m_is_correct_length() { + let actual = bech32::encoded_length::( + super::BECH32_HRP, + &[42u8; super::ADDRESS_LEN], + ) + .unwrap(); + assert_eq!(super::ADDRESS_BECH32M_LENGTH, actual); + } + + #[test] + fn mismatched_fields_in_protobuf_address_are_caught() { + // allow: deprecated code must still be tested + #![allow(deprecated)] + let bytes = [24u8; ADDRESS_LEN]; + let bech32m = [42u8; ADDRESS_LEN]; + let proto = super::raw::Address { + bytes: Bytes::copy_from_slice(&bytes), + bech32m: bech32::encode_lower::(super::BECH32_HRP, &bech32m).unwrap(), + }; + let expected = AddressErrorKind::FieldsDontMatch { + bytes, + bech32m, + }; + let actual = Address::try_from_raw(&proto) + .expect_err("returned a valid address where it should have errored"); + assert_eq!(expected, actual.0); + } + + #[test] + fn non_astria_hrp_is_caught() { + let hrp = bech32::Hrp::parse("notastria").unwrap(); + let input = bech32::encode_lower::(hrp, &[42u8; ADDRESS_LEN]).unwrap(); + let actual = Address::try_from_bech32m(&input) + .expect_err("returned a valid address where it should have errored"); + let expected = AddressErrorKind::UnknownBech32Hrp { + received: hrp, + }; + assert_eq!(expected, actual.0); + } } diff --git a/crates/astria-core/src/primitive/v1/snapshots/astria_core__primitive__v1__tests__snapshots.snap b/crates/astria-core/src/primitive/v1/snapshots/astria_core__primitive__v1__tests__snapshots.snap index 80054cbee8..10a7a57a0a 100644 --- a/crates/astria-core/src/primitive/v1/snapshots/astria_core__primitive__v1__tests__snapshots.snap +++ b/crates/astria-core/src/primitive/v1/snapshots/astria_core__primitive__v1__tests__snapshots.snap @@ -2,4 +2,7 @@ source: crates/astria-core/src/primitive/v1/mod.rs expression: address --- -"KioqKioqKioqKioqKioqKioqKio=" +{ + "bytes": "KioqKioqKioqKioqKioqKioqKio=", + "bech32m": "astria19g4z52329g4z52329g4z52329g4z5232ayenag" +} diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs index 783f237edc..3278aed432 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs @@ -16,7 +16,7 @@ use crate::{ Denom, }, Address, - IncorrectAddressLength, + AddressError, IncorrectRollupIdLength, RollupId, }, @@ -485,7 +485,7 @@ impl TransferAction { let Some(to) = to else { return Err(TransferActionError::field_not_set("to")); }; - let to = Address::try_from_raw(&to).map_err(TransferActionError::address_length)?; + let to = Address::try_from_raw(&to).map_err(TransferActionError::address)?; let amount = amount.map_or(0, Into::into); let asset_id = asset::Id::try_from_slice(&asset_id).map_err(TransferActionError::asset_id)?; @@ -510,8 +510,8 @@ impl TransferActionError { Self(TransferActionErrorKind::FieldNotSet(field)) } - fn address_length(inner: IncorrectAddressLength) -> Self { - Self(TransferActionErrorKind::AddressLength(inner)) + fn address(inner: AddressError) -> Self { + Self(TransferActionErrorKind::Address(inner)) } fn asset_id(inner: asset::IncorrectAssetIdLength) -> Self { @@ -528,7 +528,7 @@ enum TransferActionErrorKind { #[error("the expected field in the raw source type was not set: `{0}`")] FieldNotSet(&'static str), #[error("`to` field did not contain a valid address")] - AddressLength(#[source] IncorrectAddressLength), + Address(#[source] AddressError), #[error("`asset_id` field did not contain a valid asset ID")] Asset(#[source] asset::IncorrectAssetIdLength), #[error("`fee_asset_id` field did not contain a valid asset ID")] @@ -594,8 +594,10 @@ impl SudoAddressChangeActionError { Self(SudoAddressChangeActionErrorKind::FieldNotSet(field)) } - fn address(inner: IncorrectAddressLength) -> Self { - Self(SudoAddressChangeActionErrorKind::Address(inner)) + fn address(source: AddressError) -> Self { + Self(SudoAddressChangeActionErrorKind::Address { + source, + }) } } @@ -604,7 +606,7 @@ enum SudoAddressChangeActionErrorKind { #[error("the expected field in the raw source type was not set: `{0}`")] FieldNotSet(&'static str), #[error("`new_address` field did not contain a valid address")] - Address(#[source] IncorrectAddressLength), + Address { source: AddressError }, } #[allow(clippy::module_name_repetitions)] @@ -653,7 +655,7 @@ impl MintAction { let Some(to) = to else { return Err(MintActionError::field_not_set("to")); }; - let to = Address::try_from_raw(&to).map_err(MintActionError::address_length)?; + let to = Address::try_from_raw(&to).map_err(MintActionError::address)?; let amount = amount.map_or(0, Into::into); Ok(Self { to, @@ -672,8 +674,10 @@ impl MintActionError { Self(MintActionErrorKind::FieldNotSet(field)) } - fn address_length(inner: IncorrectAddressLength) -> Self { - Self(MintActionErrorKind::AddressLength(inner)) + fn address(source: AddressError) -> Self { + Self(MintActionErrorKind::Address { + source, + }) } } @@ -682,7 +686,7 @@ enum MintActionErrorKind { #[error("the expected field in the raw source type was not set: `{0}`")] FieldNotSet(&'static str), #[error("`to` field did not contain a valid address")] - AddressLength(#[source] IncorrectAddressLength), + Address { source: AddressError }, } /// Represents an IBC withdrawal of an asset from a source chain to a destination chain. @@ -815,7 +819,7 @@ impl Ics20Withdrawal { pub fn try_from_raw(proto: raw::Ics20Withdrawal) -> Result { let amount = proto.amount.ok_or(Ics20WithdrawalError::missing_amount())?; let return_address = Address::try_from_slice(&proto.return_address) - .map_err(Ics20WithdrawalError::invalid_return_address)?; + .map_err(Ics20WithdrawalError::return_address)?; let timeout_height = proto .timeout_height .ok_or(Ics20WithdrawalError::missing_timeout_height())? @@ -878,8 +882,10 @@ impl Ics20WithdrawalError { } #[must_use] - fn invalid_return_address(err: IncorrectAddressLength) -> Self { - Self(Ics20WithdrawalErrorKind::InvalidReturnAddress(err)) + fn return_address(source: AddressError) -> Self { + Self(Ics20WithdrawalErrorKind::ReturnAddress { + source, + }) } #[must_use] @@ -903,7 +909,7 @@ enum Ics20WithdrawalErrorKind { #[error("`amount` field was missing")] MissingAmount, #[error("`return_address` field was invalid")] - InvalidReturnAddress(#[source] IncorrectAddressLength), + ReturnAddress { source: AddressError }, #[error("`timeout_height` field was missing")] MissingTimeoutHeight, #[error("`source_channel` field was invalid")] @@ -964,15 +970,15 @@ impl IbcRelayerChangeAction { raw::IbcRelayerChangeAction { value: Some(raw::ibc_relayer_change_action::Value::Addition(address)), } => { - let address = Address::try_from_raw(address) - .map_err(IbcRelayerChangeActionError::invalid_address)?; + let address = + Address::try_from_raw(address).map_err(IbcRelayerChangeActionError::address)?; Ok(IbcRelayerChangeAction::Addition(address)) } raw::IbcRelayerChangeAction { value: Some(raw::ibc_relayer_change_action::Value::Removal(address)), } => { - let address = Address::try_from_raw(address) - .map_err(IbcRelayerChangeActionError::invalid_address)?; + let address = + Address::try_from_raw(address).map_err(IbcRelayerChangeActionError::address)?; Ok(IbcRelayerChangeAction::Removal(address)) } _ => Err(IbcRelayerChangeActionError::missing_address()), @@ -986,8 +992,10 @@ pub struct IbcRelayerChangeActionError(IbcRelayerChangeActionErrorKind); impl IbcRelayerChangeActionError { #[must_use] - fn invalid_address(err: IncorrectAddressLength) -> Self { - Self(IbcRelayerChangeActionErrorKind::InvalidAddress(err)) + fn address(source: AddressError) -> Self { + Self(IbcRelayerChangeActionErrorKind::Address { + source, + }) } #[must_use] @@ -998,9 +1006,9 @@ impl IbcRelayerChangeActionError { #[derive(Debug, thiserror::Error)] enum IbcRelayerChangeActionErrorKind { - #[error("the address was invalid")] - InvalidAddress(#[source] IncorrectAddressLength), - #[error("the address was missing")] + #[error("the `address` was invalid")] + Address { source: AddressError }, + #[error("the `address` was not set")] MissingAddress, } @@ -1243,7 +1251,7 @@ impl BridgeLockAction { let Some(to) = proto.to else { return Err(BridgeLockActionError::field_not_set("to")); }; - let to = Address::try_from_raw(&to).map_err(BridgeLockActionError::invalid_address)?; + let to = Address::try_from_raw(&to).map_err(BridgeLockActionError::address)?; let amount = proto .amount .ok_or(BridgeLockActionError::missing_amount())?; @@ -1272,8 +1280,10 @@ impl BridgeLockActionError { } #[must_use] - fn invalid_address(err: IncorrectAddressLength) -> Self { - Self(BridgeLockActionErrorKind::InvalidAddress(err)) + fn address(source: AddressError) -> Self { + Self(BridgeLockActionErrorKind::Address { + source, + }) } #[must_use] @@ -1297,7 +1307,7 @@ enum BridgeLockActionErrorKind { #[error("the expected field in the raw source type was not set: `{0}`")] FieldNotSet(&'static str), #[error("the `to` field was invalid")] - InvalidAddress(#[source] IncorrectAddressLength), + Address { source: AddressError }, #[error("the `amount` field was not set")] MissingAmount, #[error("the `asset_id` field was invalid")] @@ -1350,7 +1360,7 @@ impl BridgeUnlockAction { let Some(to) = proto.to else { return Err(BridgeUnlockActionError::field_not_set("to")); }; - let to = Address::try_from_raw(&to).map_err(BridgeUnlockActionError::invalid_address)?; + let to = Address::try_from_raw(&to).map_err(BridgeUnlockActionError::address)?; let amount = proto .amount .ok_or(BridgeUnlockActionError::missing_amount())?; @@ -1376,8 +1386,10 @@ impl BridgeUnlockActionError { } #[must_use] - fn invalid_address(err: IncorrectAddressLength) -> Self { - Self(BridgeUnlockActionErrorKind::InvalidAddress(err)) + fn address(source: AddressError) -> Self { + Self(BridgeUnlockActionErrorKind::Address { + source, + }) } #[must_use] @@ -1396,7 +1408,7 @@ enum BridgeUnlockActionErrorKind { #[error("the expected field in the raw source type was not set: `{0}`")] FieldNotSet(&'static str), #[error("the `to` field was invalid")] - InvalidAddress(#[source] IncorrectAddressLength), + Address { source: AddressError }, #[error("the `amount` field was not set")] MissingAmount, #[error("the `fee_asset_id` field was invalid")] diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/snapshots/astria_core__protocol__transaction__v1alpha1__test__signed_transaction_hash.snap b/crates/astria-core/src/protocol/transaction/v1alpha1/snapshots/astria_core__protocol__transaction__v1alpha1__test__signed_transaction_hash.snap index 2856a679ec..b2c38d455c 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/snapshots/astria_core__protocol__transaction__v1alpha1__test__signed_transaction_hash.snap +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/snapshots/astria_core__protocol__transaction__v1alpha1__test__signed_transaction_hash.snap @@ -3,36 +3,36 @@ source: crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs expression: tx.sha256_of_proto_encoding() --- [ - 108, - 44, - 224, - 168, - 98, - 240, - 215, + 12, + 91, 197, - 83, - 115, + 58, + 205, + 198, + 92, 206, - 104, - 180, - 159, - 248, - 74, - 224, - 209, - 176, - 215, - 243, - 132, - 197, + 230, + 47, + 37, + 22, 41, - 11, - 211, - 188, - 188, - 49, - 43, - 168, - 198 + 34, + 190, + 29, + 87, + 44, + 191, + 209, + 145, + 134, + 205, + 104, + 13, + 92, + 189, + 112, + 127, + 218, + 115, + 191 ] diff --git a/crates/astria-core/src/sequencerblock/v1alpha1/block.rs b/crates/astria-core/src/sequencerblock/v1alpha1/block.rs index 90ef2f9bfc..27c9503034 100644 --- a/crates/astria-core/src/sequencerblock/v1alpha1/block.rs +++ b/crates/astria-core/src/sequencerblock/v1alpha1/block.rs @@ -22,7 +22,7 @@ use crate::{ asset, derive_merkle_tree_from_rollup_txs, Address, - IncorrectAddressLength, + AddressError, IncorrectRollupIdLength, RollupId, }, @@ -1355,8 +1355,8 @@ impl Deposit { let Some(bridge_address) = bridge_address else { return Err(DepositError::field_not_set("bridge_address")); }; - let bridge_address = Address::try_from_raw(&bridge_address) - .map_err(DepositError::incorrect_address_length)?; + let bridge_address = + Address::try_from_raw(&bridge_address).map_err(DepositError::address)?; let amount = amount.ok_or(DepositError::field_not_set("amount"))?.into(); let Some(rollup_id) = rollup_id else { return Err(DepositError::field_not_set("rollup_id")); @@ -1380,8 +1380,10 @@ impl Deposit { pub struct DepositError(DepositErrorKind); impl DepositError { - fn incorrect_address_length(source: IncorrectAddressLength) -> Self { - Self(DepositErrorKind::IncorrectAddressLength(source)) + fn address(source: AddressError) -> Self { + Self(DepositErrorKind::Address { + source, + }) } fn field_not_set(field: &'static str) -> Self { @@ -1399,8 +1401,8 @@ impl DepositError { #[derive(Debug, thiserror::Error)] enum DepositErrorKind { - #[error("the address length is not 20 bytes")] - IncorrectAddressLength(#[source] IncorrectAddressLength), + #[error("the address is invalid")] + Address { source: AddressError }, #[error("the expected field in the raw source type was not set: `{0}`")] FieldNotSet(&'static str), #[error("the rollup ID length is not 32 bytes")] diff --git a/proto/primitives/astria/primitive/v1/types.proto b/proto/primitives/astria/primitive/v1/types.proto index c38f0c1a03..2b1603ea92 100644 --- a/proto/primitives/astria/primitive/v1/types.proto +++ b/proto/primitives/astria/primitive/v1/types.proto @@ -47,5 +47,7 @@ message RollupId { // Astria addresses are derived from the ed25519 public key, // using the first 20 bytes of the sha256 hash. message Address { - bytes inner = 1; + bytes bytes = 1 [deprecated = true]; + + string bech32m = 2; } From 7615490dd866499352ff11ac680caa6540a0c9a8 Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Thu, 30 May 2024 17:03:04 +0200 Subject: [PATCH 02/15] ensure proto changes are not breaking (json name) --- .../src/generated/astria.primitive.v1.rs | 2 +- .../generated/astria.primitive.v1.serde.rs | 24 +++++++++---------- crates/astria-core/src/primitive/v1/mod.rs | 12 +++++----- ...core__primitive__v1__tests__snapshots.snap | 2 +- .../astria/primitive/v1/types.proto | 2 +- 5 files changed, 21 insertions(+), 21 deletions(-) diff --git a/crates/astria-core/src/generated/astria.primitive.v1.rs b/crates/astria-core/src/generated/astria.primitive.v1.rs index 31d7057a35..54dbeac75c 100644 --- a/crates/astria-core/src/generated/astria.primitive.v1.rs +++ b/crates/astria-core/src/generated/astria.primitive.v1.rs @@ -88,7 +88,7 @@ impl ::prost::Name for RollupId { pub struct Address { #[deprecated] #[prost(bytes = "bytes", tag = "1")] - pub bytes: ::prost::bytes::Bytes, + pub inner: ::prost::bytes::Bytes, #[prost(string, tag = "2")] pub bech32m: ::prost::alloc::string::String, } diff --git a/crates/astria-core/src/generated/astria.primitive.v1.serde.rs b/crates/astria-core/src/generated/astria.primitive.v1.serde.rs index 90f3ca4ed4..915990f21c 100644 --- a/crates/astria-core/src/generated/astria.primitive.v1.serde.rs +++ b/crates/astria-core/src/generated/astria.primitive.v1.serde.rs @@ -6,16 +6,16 @@ impl serde::Serialize for Address { { use serde::ser::SerializeStruct; let mut len = 0; - if !self.bytes.is_empty() { + if !self.inner.is_empty() { len += 1; } if !self.bech32m.is_empty() { len += 1; } let mut struct_ser = serializer.serialize_struct("astria.primitive.v1.Address", len)?; - if !self.bytes.is_empty() { + if !self.inner.is_empty() { #[allow(clippy::needless_borrow)] - struct_ser.serialize_field("bytes", pbjson::private::base64::encode(&self.bytes).as_str())?; + struct_ser.serialize_field("inner", pbjson::private::base64::encode(&self.inner).as_str())?; } if !self.bech32m.is_empty() { struct_ser.serialize_field("bech32m", &self.bech32m)?; @@ -30,13 +30,13 @@ impl<'de> serde::Deserialize<'de> for Address { D: serde::Deserializer<'de>, { const FIELDS: &[&str] = &[ - "bytes", + "inner", "bech32m", ]; #[allow(clippy::enum_variant_names)] enum GeneratedField { - Bytes, + Inner, Bech32m, } impl<'de> serde::Deserialize<'de> for GeneratedField { @@ -59,7 +59,7 @@ impl<'de> serde::Deserialize<'de> for Address { E: serde::de::Error, { match value { - "bytes" => Ok(GeneratedField::Bytes), + "inner" => Ok(GeneratedField::Inner), "bech32m" => Ok(GeneratedField::Bech32m), _ => Err(serde::de::Error::unknown_field(value, FIELDS)), } @@ -80,15 +80,15 @@ impl<'de> serde::Deserialize<'de> for Address { where V: serde::de::MapAccess<'de>, { - let mut bytes__ = None; + let mut inner__ = None; let mut bech32m__ = None; while let Some(k) = map_.next_key()? { match k { - GeneratedField::Bytes => { - if bytes__.is_some() { - return Err(serde::de::Error::duplicate_field("bytes")); + GeneratedField::Inner => { + if inner__.is_some() { + return Err(serde::de::Error::duplicate_field("inner")); } - bytes__ = + inner__ = Some(map_.next_value::<::pbjson::private::BytesDeserialize<_>>()?.0) ; } @@ -101,7 +101,7 @@ impl<'de> serde::Deserialize<'de> for Address { } } Ok(Address { - bytes: bytes__.unwrap_or_default(), + inner: inner__.unwrap_or_default(), bech32m: bech32m__.unwrap_or_default(), }) } diff --git a/crates/astria-core/src/primitive/v1/mod.rs b/crates/astria-core/src/primitive/v1/mod.rs index e3c329200a..567b06f137 100644 --- a/crates/astria-core/src/primitive/v1/mod.rs +++ b/crates/astria-core/src/primitive/v1/mod.rs @@ -366,7 +366,7 @@ impl Address { // and should not be used by downstream users in new code. #[allow(deprecated)] raw::Address { - bytes: self.to_vec().into(), + inner: self.to_vec().into(), bech32m, } } @@ -386,16 +386,16 @@ impl Address { // and should not be used by downstream users in new code. #![allow(deprecated)] let raw::Address { - bytes, + inner, bech32m, } = raw; if bech32m.is_empty() { - return Self::try_from_slice(bytes); + return Self::try_from_slice(inner); } - if bytes.is_empty() { + if inner.is_empty() { return Self::try_from_bech32m(bech32m); } - let addr_bytes = Self::try_from_slice(bytes)?; + let addr_bytes = Self::try_from_slice(inner)?; let addr_bech32m = Self::try_from_bech32m(bech32m)?; if addr_bytes != addr_bech32m { return Err(AddressError::fields_dont_match( @@ -521,7 +521,7 @@ mod tests { let bytes = [24u8; ADDRESS_LEN]; let bech32m = [42u8; ADDRESS_LEN]; let proto = super::raw::Address { - bytes: Bytes::copy_from_slice(&bytes), + inner: Bytes::copy_from_slice(&bytes), bech32m: bech32::encode_lower::(super::BECH32_HRP, &bech32m).unwrap(), }; let expected = AddressErrorKind::FieldsDontMatch { diff --git a/crates/astria-core/src/primitive/v1/snapshots/astria_core__primitive__v1__tests__snapshots.snap b/crates/astria-core/src/primitive/v1/snapshots/astria_core__primitive__v1__tests__snapshots.snap index 10a7a57a0a..75cafa33cd 100644 --- a/crates/astria-core/src/primitive/v1/snapshots/astria_core__primitive__v1__tests__snapshots.snap +++ b/crates/astria-core/src/primitive/v1/snapshots/astria_core__primitive__v1__tests__snapshots.snap @@ -3,6 +3,6 @@ source: crates/astria-core/src/primitive/v1/mod.rs expression: address --- { - "bytes": "KioqKioqKioqKioqKioqKioqKio=", + "inner": "KioqKioqKioqKioqKioqKioqKio=", "bech32m": "astria19g4z52329g4z52329g4z52329g4z5232ayenag" } diff --git a/proto/primitives/astria/primitive/v1/types.proto b/proto/primitives/astria/primitive/v1/types.proto index 2b1603ea92..76f47ba66a 100644 --- a/proto/primitives/astria/primitive/v1/types.proto +++ b/proto/primitives/astria/primitive/v1/types.proto @@ -47,7 +47,7 @@ message RollupId { // Astria addresses are derived from the ed25519 public key, // using the first 20 bytes of the sha256 hash. message Address { - bytes bytes = 1 [deprecated = true]; + bytes inner = 1 [deprecated = true]; string bech32m = 2; } From b9ab7f3da1cd83d7e034d220f7bc9f5b0a17c5bc Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Tue, 4 Jun 2024 15:04:01 +0200 Subject: [PATCH 03/15] accept breaking changes --- ...ransaction_with_every_action_snapshot.snap | 59 +++++++++---------- ..._changes__app_finalize_block_snapshot.snap | 58 +++++++++--------- 2 files changed, 58 insertions(+), 59 deletions(-) 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 e06db4b717..045758c049 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 @@ -1,39 +1,38 @@ --- source: crates/astria-sequencer/src/app/tests_breaking_changes.rs -assertion_line: 274 expression: app.app_hash.as_bytes() --- [ - 30, 232, - 210, - 6, - 194, - 12, - 154, - 179, - 145, - 105, - 40, - 101, - 30, - 224, - 10, - 195, - 119, - 124, - 207, - 182, - 132, - 175, - 9, - 191, - 104, 213, - 200, - 146, - 180, + 217, + 84, + 137, + 197, 192, - 229, - 97 + 255, + 247, + 176, + 2, + 44, + 162, + 5, + 17, + 77, + 182, + 63, + 115, + 118, + 162, + 142, + 194, + 201, + 33, + 57, + 178, + 49, + 190, + 13, + 110, + 227 ] 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 9c81d4d7ae..11207d1607 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() --- [ - 171, - 98, - 3, - 172, - 104, - 125, - 77, - 86, - 230, - 236, - 153, - 226, - 227, - 158, + 247, + 141, 59, - 19, + 2, + 241, + 28, + 238, + 183, + 204, 108, - 136, - 6, - 56, - 175, - 113, + 89, + 90, + 223, + 104, + 190, + 38, + 228, + 148, + 207, + 177, + 52, + 34, + 126, + 52, + 50, + 206, + 92, + 48, 196, - 108, - 171, - 66, - 211, - 241, - 217, - 82, - 191, - 152 + 245, + 153, + 30 ] From 8ca0479582043255bc440342942e93b9ff5df204 Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Tue, 4 Jun 2024 15:18:55 +0200 Subject: [PATCH 04/15] add tests, remove deprecation notice on address bytes --- .../src/generated/astria.primitive.v1.rs | 1 - crates/astria-core/src/primitive/v1/mod.rs | 44 +++++++++++++++---- .../astria/primitive/v1/types.proto | 2 +- 3 files changed, 37 insertions(+), 10 deletions(-) diff --git a/crates/astria-core/src/generated/astria.primitive.v1.rs b/crates/astria-core/src/generated/astria.primitive.v1.rs index 54dbeac75c..3989442fd9 100644 --- a/crates/astria-core/src/generated/astria.primitive.v1.rs +++ b/crates/astria-core/src/generated/astria.primitive.v1.rs @@ -86,7 +86,6 @@ impl ::prost::Name for RollupId { #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct Address { - #[deprecated] #[prost(bytes = "bytes", tag = "1")] pub inner: ::prost::bytes::Bytes, #[prost(string, tag = "2")] diff --git a/crates/astria-core/src/primitive/v1/mod.rs b/crates/astria-core/src/primitive/v1/mod.rs index 567b06f137..45b021a2a7 100644 --- a/crates/astria-core/src/primitive/v1/mod.rs +++ b/crates/astria-core/src/primitive/v1/mod.rs @@ -362,9 +362,6 @@ impl Address { "must not fail because Address is tested to be ADDRESS_BECH32M_LENGTH long, which \ is less than the permitted maximum bech32m checksum length", ); - // allow: for compatibility purposes. The `bytes` protobuf field is deprecated - // and should not be used by downstream users in new code. - #[allow(deprecated)] raw::Address { inner: self.to_vec().into(), bech32m, @@ -382,9 +379,6 @@ impl Address { /// /// Returns an error if the account buffer was not 20 bytes long. pub fn try_from_raw(raw: &raw::Address) -> Result { - // allow: for compatibility purposes. The `bytes` protobuf field is deprecated - // and should not be used by downstream users in new code. - #![allow(deprecated)] let raw::Address { inner, bech32m, @@ -464,10 +458,12 @@ mod tests { use insta::assert_json_snapshot; use super::{ + raw, Address, AddressErrorKind, ADDRESS_LEN, }; + use crate::primitive::v1::BECH32_HRP; #[test] fn account_of_20_bytes_is_converted_correctly() { @@ -516,8 +512,6 @@ mod tests { #[test] fn mismatched_fields_in_protobuf_address_are_caught() { - // allow: deprecated code must still be tested - #![allow(deprecated)] let bytes = [24u8; ADDRESS_LEN]; let bech32m = [42u8; ADDRESS_LEN]; let proto = super::raw::Address { @@ -544,4 +538,38 @@ mod tests { }; assert_eq!(expected, actual.0); } + + #[test] + fn proto_with_missing_bech32m_is_accepted() { + let bytes = [42u8; ADDRESS_LEN]; + let input = raw::Address { + inner: Bytes::copy_from_slice(&bytes), + bech32m: "".into(), + }; + let address = Address::try_from_raw(&input).unwrap(); + assert_eq!(bytes, address.get()); + } + + #[test] + fn proto_with_missing_bytes_is_accepted_and_bytes_is_populated() { + let bytes = [42u8; ADDRESS_LEN]; + let input = raw::Address { + inner: Bytes::new(), + bech32m: bech32::encode_lower::(BECH32_HRP, &bytes).unwrap(), + }; + let address = Address::try_from_raw(&input).unwrap(); + assert_eq!(bytes, address.get()); + } + + #[test] + fn protobuf_has_bytes_and_bech32m_populated() { + let bytes = [42u8; ADDRESS_LEN]; + let address = Address::from_array(bytes); + let output = address.into_raw(); + assert_eq!(&bytes[..], &*output.inner); + assert_eq!( + bech32::encode_lower::(BECH32_HRP, &bytes).unwrap(), + output.bech32m + ); + } } diff --git a/proto/primitives/astria/primitive/v1/types.proto b/proto/primitives/astria/primitive/v1/types.proto index 76f47ba66a..b86a1c9b70 100644 --- a/proto/primitives/astria/primitive/v1/types.proto +++ b/proto/primitives/astria/primitive/v1/types.proto @@ -47,7 +47,7 @@ message RollupId { // Astria addresses are derived from the ed25519 public key, // using the first 20 bytes of the sha256 hash. message Address { - bytes inner = 1 [deprecated = true]; + bytes inner = 1; string bech32m = 2; } From fb522c3e2cbc4725e376db1ded275451ad96b217 Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Tue, 4 Jun 2024 15:32:16 +0200 Subject: [PATCH 05/15] clippy --- crates/astria-core/src/primitive/v1/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/astria-core/src/primitive/v1/mod.rs b/crates/astria-core/src/primitive/v1/mod.rs index 45b021a2a7..66361da126 100644 --- a/crates/astria-core/src/primitive/v1/mod.rs +++ b/crates/astria-core/src/primitive/v1/mod.rs @@ -544,7 +544,7 @@ mod tests { let bytes = [42u8; ADDRESS_LEN]; let input = raw::Address { inner: Bytes::copy_from_slice(&bytes), - bech32m: "".into(), + bech32m: String::new(), }; let address = Address::try_from_raw(&input).unwrap(); assert_eq!(bytes, address.get()); From 64a36826502fce58a53e72549afd9ddd0cc92886 Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Thu, 6 Jun 2024 18:24:55 +0200 Subject: [PATCH 06/15] make addresses prefix aware --- Cargo.lock | 1 + .../src/withdrawer/ethereum/convert.rs | 18 +- .../src/withdrawer/submitter/signer.rs | 7 +- crates/astria-cli/src/cli/sequencer.rs | 5 +- crates/astria-cli/src/commands/sequencer.rs | 5 +- crates/astria-cli/src/lib.rs | 26 ++ crates/astria-core/Cargo.toml | 1 + crates/astria-core/src/crypto.rs | 51 ++-- .../src/generated/astria.primitive.v1.rs | 1 + .../astria.protocol.transactions.v1alpha1.rs | 6 +- crates/astria-core/src/primitive/v1/mod.rs | 255 ++++++++++++------ .../protocol/transaction/v1alpha1/action.rs | 37 +-- .../src/protocol/transaction/v1alpha1/mod.rs | 4 +- .../src/extension_trait.rs | 32 ++- .../astria-sequencer-client/src/tests/http.rs | 23 +- crates/astria-sequencer/src/accounts/query.rs | 2 +- .../src/accounts/state_ext.rs | 39 ++- crates/astria-sequencer/src/api_state_ext.rs | 7 +- crates/astria-sequencer/src/app/mod.rs | 2 +- crates/astria-sequencer/src/app/test_utils.rs | 6 +- crates/astria-sequencer/src/app/tests_app.rs | 7 +- .../src/app/tests_breaking_changes.rs | 3 +- .../src/app/tests_execute_transaction.rs | 11 +- .../astria-sequencer/src/authority/action.rs | 12 +- .../src/authority/state_ext.rs | 11 +- .../src/bridge/bridge_lock_action.rs | 8 +- .../src/bridge/bridge_unlock_action.rs | 12 +- .../astria-sequencer/src/bridge/state_ext.rs | 23 +- crates/astria-sequencer/src/config.rs | 2 + crates/astria-sequencer/src/genesis.rs | 8 +- .../src/ibc/ics20_transfer.rs | 14 +- crates/astria-sequencer/src/ibc/state_ext.rs | 23 +- crates/astria-sequencer/src/lib.rs | 24 ++ crates/astria-sequencer/src/mempool.rs | 39 ++- .../src/proposal/commitment.rs | 13 +- .../astria-sequencer/src/service/consensus.rs | 7 +- .../astria-sequencer/src/service/info/mod.rs | 11 +- .../astria-sequencer/src/service/mempool.rs | 4 +- .../src/transaction/checks.rs | 8 +- .../astria-sequencer/src/transaction/mod.rs | 4 +- .../astria/primitive/v1/types.proto | 2 +- .../transactions/v1alpha1/types.proto | 2 +- 42 files changed, 458 insertions(+), 318 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 019b42f03e..1f21ad9fc2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -667,6 +667,7 @@ dependencies = [ name = "astria-core" version = "0.1.0" dependencies = [ + "arrayvec 0.7.4", "astria-core", "astria-merkle", "base64 0.21.7", diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/convert.rs b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/convert.rs index 90857f853d..24540ce0f8 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/convert.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/convert.rs @@ -95,7 +95,11 @@ fn event_to_bridge_unlock( transaction_hash, }; let action = BridgeUnlockAction { - to: event.sender.to_fixed_bytes().into(), + to: Address::builder() + .array(event.sender.to_fixed_bytes().into()) + .prefix("astria") + .try_build() + .wrap_err("failed to construct destination address")?, amount: event .amount .as_u128() @@ -152,7 +156,11 @@ fn event_to_ics20_withdrawal( // returned to the rollup. // this is only ok for now because addresses on the sequencer and the rollup are both 20 // bytes, but this won't work otherwise. - return_address: Address::from(sender), + return_address: Address::builder() + .array(sender) + .prefix("astria") + .try_build() + .wrap_err("failed to construct return address")?, amount: event .amount .as_u128() @@ -207,7 +215,7 @@ mod tests { }; let expected_action = BridgeUnlockAction { - to: [0u8; 20].into(), + to: Address::builder().array([0u8; 20]).prefix("astria").build(), amount: 99, memo: serde_json::to_vec(&BridgeUnlockMemo { block_number: 1.into(), @@ -239,7 +247,7 @@ mod tests { }; let expected_action = BridgeUnlockAction { - to: [0u8; 20].into(), + to: Address::builder().array([0u8; 20]).prefix("astria").build(), amount: 99, memo: serde_json::to_vec(&BridgeUnlockMemo { block_number: 1.into(), @@ -279,7 +287,7 @@ mod tests { let expected_action = Ics20Withdrawal { denom: denom.clone(), destination_chain_address, - return_address: [0u8; 20].into(), + return_address: Address::builder().array([0u8; 20]).prefix("astria").build(), amount: 99, memo: serde_json::to_string(&Ics20WithdrawalMemo { memo: "hello".to_string(), diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs index 853d08ff91..a5b3c5c029 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs @@ -7,6 +7,7 @@ use astria_core::crypto::SigningKey; use astria_eyre::eyre::{ self, eyre, + WrapErr as _, }; use sequencer_client::Address; @@ -27,7 +28,11 @@ impl SequencerKey { let signing_key = SigningKey::from(bytes); Ok(Self { - address: *signing_key.verification_key().address(), + address: Address::builder() + .array(signing_key.verification_key().address_bytes()) + .prefix("astria") + .try_build() + .wrap_err("failed to construct Sequencer address")?, signing_key, }) } diff --git a/crates/astria-cli/src/cli/sequencer.rs b/crates/astria-cli/src/cli/sequencer.rs index cca710530f..affb28d639 100644 --- a/crates/astria-cli/src/cli/sequencer.rs +++ b/crates/astria-cli/src/cli/sequencer.rs @@ -254,8 +254,7 @@ impl FromStr for SequencerAddressArg { "failed to decode address. address should be 20 bytes long. do not prefix with 0x", )?; let address = - Address::try_from_slice(address_bytes.as_ref()).wrap_err("failed to create address")?; - + crate::try_astria_address(&address_bytes).wrap_err("failed to create address")?; Ok(Self(address)) } } @@ -350,7 +349,7 @@ mod tests { fn test_sequencer_address_arg_from_str_valid() { let hex_str = "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6e7f8a9b0"; let bytes = hex::decode(hex_str).unwrap(); - let expected_address = Address::try_from_slice(&bytes).unwrap(); + let expected_address = crate::try_astria_address(&bytes).unwrap(); let sequencer_address_arg: SequencerAddressArg = hex_str.parse().unwrap(); assert_eq!(sequencer_address_arg, SequencerAddressArg(expected_address)); diff --git a/crates/astria-cli/src/commands/sequencer.rs b/crates/astria-cli/src/commands/sequencer.rs index 9b9460c08d..22e592a503 100644 --- a/crates/astria-cli/src/commands/sequencer.rs +++ b/crates/astria-cli/src/commands/sequencer.rs @@ -63,8 +63,7 @@ fn get_private_key_pretty(signing_key: &SigningKey) -> String { /// Get the address from the signing key fn get_address_pretty(signing_key: &SigningKey) -> String { - let address = *signing_key.verification_key().address(); - hex::encode(address.to_vec()) + hex::encode(&signing_key.verification_key().address_bytes()) } /// Generates a new ED25519 keypair and prints the public key, private key, and address @@ -444,7 +443,7 @@ async fn submit_transaction( .map_err(|_| eyre!("invalid private key length; must be 32 bytes"))?; let sequencer_key = SigningKey::from(private_key_bytes); - let from_address = *sequencer_key.verification_key().address(); + let from_address = crate::astria_address(sequencer_key.verification_key().address_bytes()); println!("sending tx from address: {from_address}"); let nonce_res = sequencer_client diff --git a/crates/astria-cli/src/lib.rs b/crates/astria-cli/src/lib.rs index 2c16dcbda6..6c787a1264 100644 --- a/crates/astria-cli/src/lib.rs +++ b/crates/astria-cli/src/lib.rs @@ -1,3 +1,29 @@ +use astria_core::primitive::v1::{ + Address, + AddressError, +}; + pub mod cli; pub mod commands; pub mod types; + +const ADDRESS_PREFIX: &str = "astria"; + +/// Constructs an [`Address`] prefixed by `"astria"`. +pub(crate) fn astria_address(array: [u8; astria_core::primitive::v1::ADDRESS_LEN]) -> Address { + Address::builder() + .array(array) + .prefix(ADDRESS_PREFIX) + .build() +} + +/// Tries to construct an [`Address`] prefixed by `"astria"` from a byte slice. +/// +/// # Errors +/// Fails if the slice does not contain 20 bytes. +pub(crate) fn try_astria_address(slice: &[u8]) -> Result { + Address::builder() + .slice(slice) + .prefix(ADDRESS_PREFIX) + .try_build() +} diff --git a/crates/astria-core/Cargo.toml b/crates/astria-core/Cargo.toml index 9d2c203915..94ed689134 100644 --- a/crates/astria-core/Cargo.toml +++ b/crates/astria-core/Cargo.toml @@ -45,6 +45,7 @@ base64-serde = { workspace = true, optional = true } base64 = { workspace = true } zeroize = { version = "1.7.0", features = ["zeroize_derive"] } bech32 = "0.11.0" +arrayvec = "0.7.4" [features] celestia = ["dep:celestia-types"] diff --git a/crates/astria-core/src/crypto.rs b/crates/astria-core/src/crypto.rs index 2c1b6a98e6..be6442e81d 100644 --- a/crates/astria-core/src/crypto.rs +++ b/crates/astria-core/src/crypto.rs @@ -10,7 +10,6 @@ use std::{ Hash, Hasher, }, - sync::OnceLock, }; use base64::{ @@ -37,10 +36,7 @@ use zeroize::{ ZeroizeOnDrop, }; -use crate::primitive::v1::{ - Address, - ADDRESS_LEN, -}; +use crate::primitive::v1::ADDRESS_LEN; /// An Ed25519 signing key. // *Implementation note*: this is currently a refinement type around @@ -78,7 +74,6 @@ impl SigningKey { pub fn verification_key(&self) -> VerificationKey { VerificationKey { key: self.0.verification_key(), - address: OnceLock::new(), } } } @@ -116,10 +111,6 @@ impl From<[u8; 32]> for SigningKey { #[derive(Clone)] pub struct VerificationKey { key: Ed25519VerificationKey, - // The address is lazily-initialized. Since it may or may not be initialized for any given - // instance of a verification key, it is excluded from `PartialEq`, `Eq`, `PartialOrd`, `Ord` - // and `Hash` impls. - address: OnceLock
, } impl VerificationKey { @@ -147,28 +138,27 @@ impl VerificationKey { // Silence the clippy lint because the function body asserts that the panic // cannot happen. #[allow(clippy::missing_panics_doc)] - pub fn address(&self) -> &Address { - self.address.get_or_init(|| { - /// this ensures that `ADDRESS_LEN` is never accidentally changed to a value - /// that would violate this assumption. - #[allow(clippy::assertions_on_constants)] - const _: () = assert!(ADDRESS_LEN <= 32); - let bytes: [u8; 32] = Sha256::digest(self).into(); - Address::try_from_slice(&bytes[..ADDRESS_LEN]) - .expect("can convert 32 byte hash to 20 byte array") - }) + pub fn address_bytes(&self) -> [u8; ADDRESS_LEN] { + fn first_20(array: [u8; 32]) -> [u8; ADDRESS_LEN] { + [ + array[0], array[1], array[2], array[3], array[4], array[5], array[6], array[7], + array[8], array[9], array[10], array[11], array[12], array[13], array[14], + array[15], array[16], array[17], array[18], array[19], + ] + } + /// this ensures that `ADDRESS_LEN` is never accidentally changed to a value + /// that would violate this assumption. + #[allow(clippy::assertions_on_constants)] + const _: () = assert!(ADDRESS_LEN <= 32); + let bytes: [u8; 32] = Sha256::digest(self).into(); + first_20(bytes) } } impl Debug for VerificationKey { fn fmt(&self, formatter: &mut Formatter<'_>) -> fmt::Result { - let mut debug_struct = formatter.debug_struct("VerifyingKey"); + let mut debug_struct = formatter.debug_struct("VerificationKey"); debug_struct.field("key", &BASE64_STANDARD.encode(self.key.as_ref())); - if let Some(address) = self.address.get() { - debug_struct.field("address", address); - } else { - debug_struct.field("address", &"unset"); - } debug_struct.finish() } } @@ -218,7 +208,6 @@ impl TryFrom<&[u8]> for VerificationKey { let key = Ed25519VerificationKey::try_from(slice)?; Ok(Self { key, - address: OnceLock::new(), }) } } @@ -230,7 +219,6 @@ impl TryFrom<[u8; 32]> for VerificationKey { let key = Ed25519VerificationKey::try_from(bytes)?; Ok(Self { key, - address: OnceLock::new(), }) } } @@ -291,22 +279,18 @@ mod tests { // A key which compares greater than "low" ones below, and with its address uninitialized. let high_uninit = VerificationKey { key: SigningKey::from([255; 32]).0.verification_key(), - address: OnceLock::new(), }; // A key equal to `high_uninit`, but with its address initialized. let high_init = VerificationKey { key: high_uninit.key, - address: OnceLock::from(Address::from([255; 20])), }; // A key which compares less than "high" ones above, and with its address uninitialized. let low_uninit = VerificationKey { key: SigningKey::from([0; 32]).0.verification_key(), - address: OnceLock::new(), }; // A key equal to `low_uninit`, but with its address initialized. let low_init = VerificationKey { key: low_uninit.key, - address: OnceLock::from(Address::from([255; 20])), }; assert!(high_uninit.cmp(&high_uninit) == Ordering::Equal); @@ -412,15 +396,12 @@ mod tests { // Check verification keys compare equal if and only if their keys are equal. let key0 = VerificationKey { key: SigningKey::from([0; 32]).0.verification_key(), - address: OnceLock::new(), }; let other_key0 = VerificationKey { key: SigningKey::from([0; 32]).0.verification_key(), - address: OnceLock::from(Address::from([0; 20])), }; let key1 = VerificationKey { key: SigningKey::from([1; 32]).0.verification_key(), - address: OnceLock::new(), }; assert!(key0 == other_key0); diff --git a/crates/astria-core/src/generated/astria.primitive.v1.rs b/crates/astria-core/src/generated/astria.primitive.v1.rs index 3989442fd9..54dbeac75c 100644 --- a/crates/astria-core/src/generated/astria.primitive.v1.rs +++ b/crates/astria-core/src/generated/astria.primitive.v1.rs @@ -86,6 +86,7 @@ impl ::prost::Name for RollupId { #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct Address { + #[deprecated] #[prost(bytes = "bytes", tag = "1")] pub inner: ::prost::bytes::Bytes, #[prost(string, tag = "2")] 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 09b82f6408..db19dfc0a7 100644 --- a/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.rs +++ b/crates/astria-core/src/generated/astria.protocol.transactions.v1alpha1.rs @@ -186,8 +186,10 @@ pub struct Ics20Withdrawal { pub destination_chain_address: ::prost::alloc::string::String, /// an Astria address to use to return funds from this withdrawal /// in the case it fails. - #[prost(bytes = "vec", tag = "4")] - pub return_address: ::prost::alloc::vec::Vec, + #[prost(message, optional, tag = "4")] + pub return_address: ::core::option::Option< + super::super::super::primitive::v1::Address, + >, /// the height (on Astria) at which this transfer expires. #[prost(message, optional, tag = "5")] pub timeout_height: ::core::option::Option, diff --git a/crates/astria-core/src/primitive/v1/mod.rs b/crates/astria-core/src/primitive/v1/mod.rs index 66361da126..f8da0726d9 100644 --- a/crates/astria-core/src/primitive/v1/mod.rs +++ b/crates/astria-core/src/primitive/v1/mod.rs @@ -8,6 +8,7 @@ use base64::{ BASE64_STANDARD, }, }; +use bytes::Bytes; use sha2::{ Digest as _, Sha256, @@ -265,7 +266,13 @@ impl AddressError { }) } - fn fields_dont_match(bytes: [u8; ADDRESS_LEN], bech32m: [u8; ADDRESS_LEN]) -> Self { + fn bech32_hrp_too_long(source: arrayvec::CapacityError) -> Self { + Self(AddressErrorKind::Bech32HrpTooLong { + source, + }) + } + + fn fields_dont_match(bytes: Bytes, bech32m: [u8; ADDRESS_LEN]) -> Self { Self(AddressErrorKind::FieldsDontMatch { bytes, bech32m, @@ -277,12 +284,6 @@ impl AddressError { received, }) } - - fn unknown_bech32_hrp(received: bech32::Hrp) -> Self { - Self(AddressErrorKind::UnknownBech32Hrp { - received, - }) - } } #[derive(Debug, thiserror::Error, PartialEq)] @@ -295,40 +296,115 @@ enum AddressErrorKind { BASE64_STANDARD.encode(.bech32m), )] FieldsDontMatch { - bytes: [u8; ADDRESS_LEN], + bytes: Bytes, bech32m: [u8; ADDRESS_LEN], }, #[error("expected an address of 20 bytes, got `{received}`")] IncorrectAddressLength { received: usize }, - #[error("expected `\"astria\"` as the bech32 human readable prefix, got `\"{received}\"`")] - UnknownBech32Hrp { received: bech32::Hrp }, + #[error( + "the bech32 human readable prefix exceeds the maximum permitted address capacity of 16 \ + bytes" + )] + Bech32HrpTooLong { source: arrayvec::CapacityError }, +} + +pub struct NoBytes; +pub struct NoPrefix; +pub struct WithBytes<'a>(BytesInner<'a>); +enum BytesInner<'a> { + Array([u8; ADDRESS_LEN]), + Slice(std::borrow::Cow<'a, [u8]>), +} +pub struct WithPrefix<'a>(std::borrow::Cow<'a, str>); + +pub struct AddressBuilder { + bytes: TBytes, + prefix: TPrefix, +} + +impl AddressBuilder { + const fn new() -> Self { + Self { + bytes: NoBytes, + prefix: NoPrefix, + } + } +} + +impl AddressBuilder { + pub fn array(self, array: [u8; ADDRESS_LEN]) -> AddressBuilder, TPrefix> { + AddressBuilder { + bytes: WithBytes(BytesInner::Array(array)), + prefix: self.prefix, + } + } + + pub fn slice<'a, T: Into>>( + self, + bytes: T, + ) -> AddressBuilder, TPrefix> { + AddressBuilder { + bytes: WithBytes(BytesInner::Slice(bytes.into())), + prefix: self.prefix, + } + } + + pub fn prefix<'a, T: Into>>( + self, + prefix: T, + ) -> AddressBuilder> { + AddressBuilder { + bytes: self.bytes, + prefix: WithPrefix(prefix.into()), + } + } +} + +impl<'a, 'b> AddressBuilder, WithPrefix<'b>> { + pub fn build(self) -> Address { + self.try_build().unwrap() + } + + pub fn try_build(self) -> Result { + let Self { + bytes: WithBytes(bytes), + prefix: WithPrefix(prefix), + } = self; + let bytes = match bytes { + BytesInner::Array(bytes) => bytes, + BytesInner::Slice(bytes) => <[u8; ADDRESS_LEN]>::try_from(bytes.as_ref()) + .map_err(|_| AddressError::incorrect_address_length(bytes.len()))?, + }; + let prefix = arrayvec::ArrayString::from(prefix.as_ref()) + .map_err(arrayvec::CapacityError::simplify) + .map_err(AddressError::bech32_hrp_too_long)?; + Ok(Address { + bytes, + prefix, + }) + } } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] #[cfg_attr(feature = "serde", derive(serde::Serialize))] #[cfg_attr(feature = "serde", serde(into = "raw::Address"))] -pub struct Address([u8; ADDRESS_LEN]); +pub struct Address { + prefix: arrayvec::ArrayString<16>, + bytes: [u8; ADDRESS_LEN], +} impl Address { - #[must_use] - pub fn get(self) -> [u8; ADDRESS_LEN] { - self.0 + pub fn builder() -> AddressBuilder { + AddressBuilder::new() } + // #[must_use] + // pub fn get(self) -> [u8; ADDRESS_LEN] { + // self.0 + // } #[must_use] - pub fn to_vec(&self) -> Vec { - self.0.to_vec() - } - - /// Convert a byte slice to an address. - /// - /// # Errors - /// - /// Returns an error if the account buffer was not 20 bytes long. - pub fn try_from_slice(bytes: &[u8]) -> Result { - let inner = <[u8; ADDRESS_LEN]>::try_from(bytes) - .map_err(|_| AddressError::incorrect_address_length(bytes.len()))?; - Ok(Self::from_array(inner)) + pub fn bytes(self) -> [u8; ADDRESS_LEN] { + self.bytes } /// Convert a string containing a bech32m string to an astria address. @@ -336,19 +412,14 @@ impl Address { /// # Errors /// Returns an error if: /// + `input` is not bech32m encoded. - /// + the human readable prefix (bech32 HRP) is not `"astria"`. /// + the decoded data contained in `input` is not 20 bytes long. + /// + the bech32 hrp prefix exceeds 16 bytes. pub fn try_from_bech32m(input: &str) -> Result { let (hrp, bytes) = bech32::decode(input).map_err(AddressError::bech32m_decode)?; - if hrp != BECH32_HRP { - return Err(AddressError::unknown_bech32_hrp(hrp)); - } - Self::try_from_slice(&bytes) - } - - #[must_use] - pub const fn from_array(array: [u8; ADDRESS_LEN]) -> Self { - Self(array) + Self::builder() + .slice(bytes) + .prefix(hrp.as_str()) + .try_build() } /// Convert [`Address`] to a [`raw::Address`]. @@ -357,13 +428,15 @@ impl Address { #[must_use] pub fn to_raw(&self) -> raw::Address { let mut bech32m = String::with_capacity(ADDRESS_BECH32M_LENGTH); - bech32::encode_lower_to_fmt::(&mut bech32m, BECH32_HRP, &self.get()) + bech32::encode_lower_to_fmt::(&mut bech32m, BECH32_HRP, &self.bytes()) .expect( "must not fail because Address is tested to be ADDRESS_BECH32M_LENGTH long, which \ is less than the permitted maximum bech32m checksum length", ); + // allow: the field is deprecated, but we must still fill it in + #[allow(deprecated)] raw::Address { - inner: self.to_vec().into(), + inner: Bytes::copy_from_slice(&self.bytes()), bech32m, } } @@ -379,39 +452,43 @@ impl Address { /// /// Returns an error if the account buffer was not 20 bytes long. pub fn try_from_raw(raw: &raw::Address) -> Result { + // allow: `Address::inner` field is deprecated, but we must still check it + #[allow(deprecated)] let raw::Address { inner, bech32m, } = raw; if bech32m.is_empty() { - return Self::try_from_slice(inner); + return Self::builder() + .slice(inner.as_ref()) + .prefix("astria") + .try_build(); } if inner.is_empty() { return Self::try_from_bech32m(bech32m); } - let addr_bytes = Self::try_from_slice(inner)?; - let addr_bech32m = Self::try_from_bech32m(bech32m)?; - if addr_bytes != addr_bech32m { + let address = Self::try_from_bech32m(bech32m)?; + if inner.as_ref() != &address.bytes { return Err(AddressError::fields_dont_match( - addr_bytes.get(), - addr_bech32m.get(), + inner.clone(), + address.bytes(), )); } - Ok(addr_bytes) + Ok(address) } } impl AsRef<[u8]> for Address { fn as_ref(&self) -> &[u8] { - &self.0 + &self.bytes } } -impl From<[u8; ADDRESS_LEN]> for Address { - fn from(inner: [u8; ADDRESS_LEN]) -> Self { - Self(inner) - } -} +// impl From<[u8; ADDRESS_LEN]> for Address { +// fn from(inner: [u8; ADDRESS_LEN]) -> Self { +// Self(inner) +// } +// } impl From
for raw::Address { fn from(value: Address) -> Self { @@ -422,7 +499,7 @@ impl From
for raw::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, BECH32_HRP, &self.get()) { + match bech32::encode_lower_to_fmt::(f, BECH32_HRP, &self.bytes()) { Ok(()) => Ok(()), Err(EncodeError::Fmt(err)) => Err(err), Err(err) => panic!( @@ -460,37 +537,42 @@ mod tests { use super::{ raw, Address, + AddressError, AddressErrorKind, ADDRESS_LEN, }; use crate::primitive::v1::BECH32_HRP; - #[test] - fn account_of_20_bytes_is_converted_correctly() { - let expected = Address([42; 20]); - let account_vec = expected.0.to_vec(); - let actual = Address::try_from_slice(&account_vec).unwrap(); - assert_eq!(expected, actual); - } - #[track_caller] - fn account_conversion_check(bad_account: &[u8]) { - Address::try_from_slice(bad_account).expect_err( - "converting from an incorrectly sized byte slice succeeded where it should have failed", - ); + fn assert_wrong_address_bytes(bad_account: &[u8]) { + let error = Address::builder() + .slice(bad_account) + .prefix("astria") + .try_build() + .expect_err( + "converting from an incorrectly sized byte slice succeeded where it should have \ + failed", + ); + let AddressError(AddressErrorKind::IncorrectAddressLength { + received, + }) = error + else { + panic!("expected AddressErrorKind::IncorrectAddressLength, got {error:?}"); + }; + assert_eq!(bad_account.len(), received); } #[test] fn account_of_incorrect_length_gives_error() { - account_conversion_check(&[42; 0]); - account_conversion_check(&[42; 19]); - account_conversion_check(&[42; 21]); - account_conversion_check(&[42; 100]); + assert_wrong_address_bytes(&[42; 0]); + assert_wrong_address_bytes(&[42; 19]); + assert_wrong_address_bytes(&[42; 21]); + assert_wrong_address_bytes(&[42; 100]); } #[test] fn snapshots() { - let address = Address([42; 20]); + let address = Address::builder().array([42; 20]).prefix("astria").build(); assert_json_snapshot!(address); } @@ -512,10 +594,12 @@ mod tests { #[test] fn mismatched_fields_in_protobuf_address_are_caught() { - let bytes = [24u8; ADDRESS_LEN]; + // allow: `Address::inner` field is deprecated, but we must still check it + #![allow(deprecated)] + let bytes = Bytes::copy_from_slice(&[24u8; ADDRESS_LEN]); let bech32m = [42u8; ADDRESS_LEN]; let proto = super::raw::Address { - inner: Bytes::copy_from_slice(&bytes), + inner: bytes.clone(), bech32m: bech32::encode_lower::(super::BECH32_HRP, &bech32m).unwrap(), }; let expected = AddressErrorKind::FieldsDontMatch { @@ -528,43 +612,38 @@ mod tests { } #[test] - fn non_astria_hrp_is_caught() { - let hrp = bech32::Hrp::parse("notastria").unwrap(); - let input = bech32::encode_lower::(hrp, &[42u8; ADDRESS_LEN]).unwrap(); - let actual = Address::try_from_bech32m(&input) - .expect_err("returned a valid address where it should have errored"); - let expected = AddressErrorKind::UnknownBech32Hrp { - received: hrp, - }; - assert_eq!(expected, actual.0); - } - - #[test] - fn proto_with_missing_bech32m_is_accepted() { + fn proto_with_missing_bech32m_is_accepted_and_assumed_astria() { + // allow: `Address::inner` field is deprecated, but we must still check it + #![allow(deprecated)] let bytes = [42u8; ADDRESS_LEN]; let input = raw::Address { inner: Bytes::copy_from_slice(&bytes), bech32m: String::new(), }; let address = Address::try_from_raw(&input).unwrap(); - assert_eq!(bytes, address.get()); + assert_eq!("astria", &address.prefix); + assert_eq!(bytes, address.bytes()); } #[test] fn proto_with_missing_bytes_is_accepted_and_bytes_is_populated() { + // allow: `Address::inner` field is deprecated, but we must still check it + #![allow(deprecated)] let bytes = [42u8; ADDRESS_LEN]; let input = raw::Address { inner: Bytes::new(), bech32m: bech32::encode_lower::(BECH32_HRP, &bytes).unwrap(), }; let address = Address::try_from_raw(&input).unwrap(); - assert_eq!(bytes, address.get()); + assert_eq!(bytes, address.bytes()); } #[test] fn protobuf_has_bytes_and_bech32m_populated() { + // allow: `Address::inner` field is deprecated, but we must still check it + #![allow(deprecated)] let bytes = [42u8; ADDRESS_LEN]; - let address = Address::from_array(bytes); + let address = Address::builder().array(bytes).prefix("astria").build(); let output = address.into_raw(); assert_eq!(&bytes[..], &*output.inner); assert_eq!( diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs index 6ea841d0f4..580573a60e 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/action.rs @@ -685,7 +685,7 @@ impl Ics20Withdrawal { amount: Some(self.amount.into()), denom: self.denom.to_string(), destination_chain_address: self.destination_chain_address.clone(), - return_address: self.return_address.to_vec(), + return_address: Some(self.return_address.into_raw()), timeout_height: Some(self.timeout_height.into_raw()), timeout_time: self.timeout_time, source_channel: self.source_channel.to_string(), @@ -700,7 +700,7 @@ impl Ics20Withdrawal { amount: Some(self.amount.into()), denom: self.denom.to_string(), destination_chain_address: self.destination_chain_address, - return_address: self.return_address.to_vec(), + return_address: Some(self.return_address.into_raw()), timeout_height: Some(self.timeout_height.into_raw()), timeout_time: self.timeout_time, source_channel: self.source_channel.to_string(), @@ -715,16 +715,22 @@ impl Ics20Withdrawal { /// /// - if the `amount` field is missing /// - if the `denom` field is invalid - /// - if the `return_address` field is invalid + /// - if the `return_address` field is invalid or missing /// - if the `timeout_height` field is missing /// - if the `source_channel` field is invalid pub fn try_from_raw(proto: raw::Ics20Withdrawal) -> Result { - let amount = proto.amount.ok_or(Ics20WithdrawalError::missing_amount())?; - let return_address = Address::try_from_slice(&proto.return_address) - .map_err(Ics20WithdrawalError::return_address)?; + let amount = proto + .amount + .ok_or(Ics20WithdrawalError::field_not_set("amount"))?; + let return_address = Address::try_from_raw( + &proto + .return_address + .ok_or(Ics20WithdrawalError::field_not_set("return_address"))?, + ) + .map_err(Ics20WithdrawalError::return_address)?; let timeout_height = proto .timeout_height - .ok_or(Ics20WithdrawalError::missing_timeout_height())? + .ok_or(Ics20WithdrawalError::field_not_set("timeout_height"))? .into(); Ok(Self { @@ -779,8 +785,10 @@ pub struct Ics20WithdrawalError(Ics20WithdrawalErrorKind); impl Ics20WithdrawalError { #[must_use] - fn missing_amount() -> Self { - Self(Ics20WithdrawalErrorKind::MissingAmount) + fn field_not_set(field: &'static str) -> Self { + Self(Ics20WithdrawalErrorKind::FieldNotSet { + field, + }) } #[must_use] @@ -790,11 +798,6 @@ impl Ics20WithdrawalError { }) } - #[must_use] - fn missing_timeout_height() -> Self { - Self(Ics20WithdrawalErrorKind::MissingTimeoutHeight) - } - #[must_use] fn invalid_source_channel(err: IdentifierError) -> Self { Self(Ics20WithdrawalErrorKind::InvalidSourceChannel(err)) @@ -808,12 +811,10 @@ impl Ics20WithdrawalError { #[derive(Debug, thiserror::Error)] enum Ics20WithdrawalErrorKind { - #[error("`amount` field was missing")] - MissingAmount, + #[error("expected field `{field}` was not set`")] + FieldNotSet { field: &'static str }, #[error("`return_address` field was invalid")] ReturnAddress { source: AddressError }, - #[error("`timeout_height` field was missing")] - MissingTimeoutHeight, #[error("`source_channel` field was invalid")] InvalidSourceChannel(#[source] IdentifierError), #[error("`fee_asset_id` field was invalid")] diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs index 25a0d5dcb4..4a212e73bf 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs @@ -416,7 +416,7 @@ mod test { ]); let transfer = TransferAction { - to: Address::from([0; 20]), + to: Address::builder().array([0; 20]).prefix("astria").build(), amount: 0, asset_id: default_native_asset_id(), fee_asset_id: default_native_asset_id(), @@ -449,7 +449,7 @@ mod test { ]); let transfer = TransferAction { - to: Address::from([0; 20]), + to: Address::builder().array([0; 20]).prefix("astria").build(), amount: 0, asset_id: default_native_asset_id(), fee_asset_id: default_native_asset_id(), diff --git a/crates/astria-sequencer-client/src/extension_trait.rs b/crates/astria-sequencer-client/src/extension_trait.rs index b69da4fa94..ffaa6ee065 100644 --- a/crates/astria-sequencer-client/src/extension_trait.rs +++ b/crates/astria-sequencer-client/src/extension_trait.rs @@ -8,11 +8,17 @@ //! The example below works with the feature `"http"` set. //! ```no_run //! # tokio_test::block_on(async { +//! use astria_core::primitive::v1::Address; //! use astria_sequencer_client::SequencerClientExt as _; //! use tendermint_rpc::HttpClient; //! //! let client = HttpClient::new("http://127.0.0.1:26657")?; -//! let address: [u8; 20] = hex_literal::hex!("DEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF"); +//! let address = Address::builder() +//! .array(hex_literal::hex!( +//! "DEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF" +//! )) +//! .prefix("astria") +//! .build(); //! let height = 5u32; //! let balance = client.get_balance(address, height).await?; //! println!("{balance:?}"); @@ -390,18 +396,17 @@ pub trait SequencerClientExt: Client { /// - If calling tendermint `abci_query` RPC fails. /// - If the bytes contained in the abci query response cannot be read as an /// `astria.sequencer.v1.BalanceResponse`. - async fn get_balance( + async fn get_balance( &self, - address: AddressT, + address: Address, height: HeightT, ) -> Result where - AddressT: Into
+ Send, HeightT: Into + Send, { const PREFIX: &[u8] = b"accounts/balance/"; - let path = make_path_from_prefix_and_address(PREFIX, address.into().get()); + let path = make_path_from_prefix_and_address(PREFIX, address.bytes()); let response = self .abci_query(Some(path), vec![], Some(height.into()), false) @@ -427,10 +432,7 @@ pub trait SequencerClientExt: Client { /// # Errors /// /// This has the same error conditions as [`SequencerClientExt::get_balance`]. - async fn get_latest_balance + Send>( - &self, - address: A, - ) -> Result { + async fn get_latest_balance(&self, address: Address) -> Result { // This makes use of the fact that a height `None` and `Some(0)` are // treated the same. self.get_balance(address, 0u32).await @@ -443,18 +445,17 @@ pub trait SequencerClientExt: Client { /// - If calling tendermint `abci_query` RPC fails. /// - If the bytes contained in the abci query response cannot be read as an /// `astria.sequencer.v1.NonceResponse`. - async fn get_nonce( + async fn get_nonce( &self, - address: AddressT, + address: Address, height: HeightT, ) -> Result where - AddressT: Into
+ Send, HeightT: Into + Send, { const PREFIX: &[u8] = b"accounts/nonce/"; - let path = make_path_from_prefix_and_address(PREFIX, address.into().get()); + let path = make_path_from_prefix_and_address(PREFIX, address.bytes()); let response = self .abci_query(Some(path), vec![], Some(height.into()), false) @@ -476,10 +477,7 @@ pub trait SequencerClientExt: Client { /// # Errors /// /// This has the same error conditions as [`SequencerClientExt::get_nonce`]. - async fn get_latest_nonce + Send>( - &self, - address: A, - ) -> Result { + async fn get_latest_nonce(&self, address: Address) -> Result { // This makes use of the fact that a height `None` and `Some(0)` are // treated the same. self.get_nonce(address, 0u32).await diff --git a/crates/astria-sequencer-client/src/tests/http.rs b/crates/astria-sequencer-client/src/tests/http.rs index 34d2b6153d..8701ffe6e8 100644 --- a/crates/astria-sequencer-client/src/tests/http.rs +++ b/crates/astria-sequencer-client/src/tests/http.rs @@ -39,8 +39,21 @@ use crate::{ SequencerClientExt as _, }; -const ALICE_ADDRESS: [u8; 20] = hex!("1c0c490f1b5528d8173c5de46d131160e4b2c0c3"); -const BOB_ADDRESS: Address = Address::from_array(hex!("34fec43c7fcab9aef3b3cf8aba855e41ee69ca3a")); +const ALICE_ADDRESS_BYTES: [u8; 20] = hex!("1c0c490f1b5528d8173c5de46d131160e4b2c0c3"); +const BOB_ADDRESS_BYTES: [u8; 20] = hex!("34fec43c7fcab9aef3b3cf8aba855e41ee69ca3a"); + +fn alice_address() -> Address { + Address::builder() + .array(ALICE_ADDRESS_BYTES) + .prefix("astria") + .build() +} +fn bob_address() -> Address { + Address::builder() + .array(BOB_ADDRESS_BYTES) + .prefix("astria") + .build() +} struct MockSequencer { server: MockServer, @@ -130,7 +143,7 @@ fn create_signed_transaction() -> SignedTransaction { let actions = vec![ TransferAction { - to: BOB_ADDRESS, + to: bob_address(), amount: 333_333, asset_id: default_native_asset_id(), fee_asset_id: default_native_asset_id(), @@ -163,7 +176,7 @@ async fn get_latest_nonce() { register_abci_query_response(&server, "accounts/nonce/", expected_response.clone()).await; let actual_response = client - .get_latest_nonce(ALICE_ADDRESS) + .get_latest_nonce(alice_address()) .await .unwrap() .into_raw(); @@ -193,7 +206,7 @@ async fn get_latest_balance() { register_abci_query_response(&server, "accounts/balance/", expected_response.clone()).await; let actual_response = client - .get_latest_balance(ALICE_ADDRESS) + .get_latest_balance(alice_address()) .await .unwrap() .into_raw(); diff --git a/crates/astria-sequencer/src/accounts/query.rs b/crates/astria-sequencer/src/accounts/query.rs index 2ac08081b1..c3cfb259c4 100644 --- a/crates/astria-sequencer/src/accounts/query.rs +++ b/crates/astria-sequencer/src/accounts/query.rs @@ -143,7 +143,7 @@ async fn preprocess_request( let address = hex::decode(address) .context("failed decoding hex encoded bytes") .and_then(|addr| { - Address::try_from_slice(&addr).context("failed constructing address from bytes") + crate::try_astria_address(&addr).context("failed constructing address from bytes") }) .map_err(|err| response::Query { code: AbciErrorCode::INVALID_PARAMETER.into(), diff --git a/crates/astria-sequencer/src/accounts/state_ext.rs b/crates/astria-sequencer/src/accounts/state_ext.rs index b1053ea602..138ba985a1 100644 --- a/crates/astria-sequencer/src/accounts/state_ext.rs +++ b/crates/astria-sequencer/src/accounts/state_ext.rs @@ -232,13 +232,10 @@ impl StateWriteExt for T {} #[cfg(test)] mod test { use astria_core::{ - primitive::v1::{ - asset::{ - Denom, - Id, - DEFAULT_NATIVE_ASSET_DENOM, - }, - Address, + primitive::v1::asset::{ + Denom, + Id, + DEFAULT_NATIVE_ASSET_DENOM, }, protocol::account::v1alpha1::AssetBalance, }; @@ -257,7 +254,7 @@ mod test { let state = StateDelta::new(snapshot); // create needed variables - let address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let address = crate::astria_address([42u8; 20]); let nonce_expected = 0u32; // uninitialized accounts return zero @@ -278,7 +275,7 @@ mod test { let mut state = StateDelta::new(snapshot); // create needed variables - let address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let address = crate::astria_address([42u8; 20]); let nonce_expected = 0u32; // can write new @@ -316,7 +313,7 @@ mod test { let mut state = StateDelta::new(snapshot); // create needed variables - let address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let address = crate::astria_address([42u8; 20]); let nonce_expected = 2u32; // can write new @@ -333,7 +330,7 @@ mod test { ); // writing additional account preserves first account's values - let address_1 = Address::try_from_slice(&[41u8; 20]).unwrap(); + let address_1 = crate::astria_address([41u8; 20]); let nonce_expected_1 = 3u32; state @@ -364,7 +361,7 @@ mod test { let state = StateDelta::new(snapshot); // create needed variables - let address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let address = crate::astria_address([42u8; 20]); let asset = Id::from_denom("asset_0"); let amount_expected = 0u128; @@ -386,7 +383,7 @@ mod test { let mut state = StateDelta::new(snapshot); // create needed variables - let address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let address = crate::astria_address([42u8; 20]); let asset = Id::from_denom("asset_0"); let mut amount_expected = 1u128; @@ -428,7 +425,7 @@ mod test { let mut state = StateDelta::new(snapshot); // create needed variables - let address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let address = crate::astria_address([42u8; 20]); let asset = Id::from_denom("asset_0"); let amount_expected = 1u128; @@ -448,7 +445,7 @@ mod test { // writing to other accounts does not affect original account // create needed variables - let address_1 = Address::try_from_slice(&[41u8; 20]).unwrap(); + let address_1 = crate::astria_address([41u8; 20]); let amount_expected_1 = 2u128; state @@ -481,7 +478,7 @@ mod test { let mut state = StateDelta::new(snapshot); // create needed variables - let address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let address = crate::astria_address([42u8; 20]); let asset_0 = Id::from_denom("asset_0"); let asset_1 = Id::from_denom("asset_1"); let amount_expected_0 = 1u128; @@ -520,7 +517,7 @@ mod test { let state = StateDelta::new(snapshot); // create needed variables - let address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let address = crate::astria_address([42u8; 20]); // see that call was ok let balances = state @@ -564,7 +561,7 @@ mod test { .expect("should be able to call other trait method on state object"); // create needed variables - let address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let address = crate::astria_address([42u8; 20]); let amount_expected_0 = 1u128; let amount_expected_1 = 2u128; let amount_expected_2 = 3u128; @@ -611,7 +608,7 @@ mod test { let mut state = StateDelta::new(snapshot); // create needed variables - let address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let address = crate::astria_address([42u8; 20]); let asset = Id::from_denom("asset_0"); let amount_increase = 2u128; @@ -652,7 +649,7 @@ mod test { let mut state = StateDelta::new(snapshot); // create needed variables - let address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let address = crate::astria_address([42u8; 20]); let asset = Id::from_denom("asset_0"); let amount_increase = 2u128; @@ -694,7 +691,7 @@ mod test { let mut state = StateDelta::new(snapshot); // create needed variables - let address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let address = crate::astria_address([42u8; 20]); let asset = Id::from_denom("asset_0"); let amount_increase = 2u128; diff --git a/crates/astria-sequencer/src/api_state_ext.rs b/crates/astria-sequencer/src/api_state_ext.rs index c7cc067f2a..57880c2624 100644 --- a/crates/astria-sequencer/src/api_state_ext.rs +++ b/crates/astria-sequencer/src/api_state_ext.rs @@ -383,10 +383,7 @@ impl StateWriteExt for T {} #[cfg(test)] mod test { use astria_core::{ - primitive::v1::{ - asset::Id, - Address, - }, + primitive::v1::asset::Id, protocol::test_utils::ConfigureSequencerBlock, sequencerblock::v1alpha1::block::Deposit, }; @@ -404,7 +401,7 @@ mod test { let mut deposits = vec![]; for _ in 0..2 { let rollup_id = RollupId::new(rng.gen()); - let bridge_address = Address::try_from_slice(&[rng.gen(); 20]).unwrap(); + let bridge_address = crate::astria_address([rng.gen(); 20]); let amount = rng.gen::(); let asset_id = Id::from_denom(&rng.gen::().to_string()); let destination_chain_address = rng.gen::().to_string(); diff --git a/crates/astria-sequencer/src/app/mod.rs b/crates/astria-sequencer/src/app/mod.rs index bff3d5764c..b00c02b9ec 100644 --- a/crates/astria-sequencer/src/app/mod.rs +++ b/crates/astria-sequencer/src/app/mod.rs @@ -961,7 +961,7 @@ impl App { /// Executes a signed transaction. #[instrument(name = "App::execute_transaction", skip_all, fields( signed_transaction_hash = %telemetry::display::base64(&signed_tx.sha256_of_proto_encoding()), - sender = %signed_tx.verification_key().address(), + sender = %telemetry::display::base64(&signed_tx.verification_key().address_bytes()), ))] pub(crate) async fn execute_transaction( &mut self, diff --git a/crates/astria-sequencer/src/app/test_utils.rs b/crates/astria-sequencer/src/app/test_utils.rs index 09c42c2f01..6ea666eb29 100644 --- a/crates/astria-sequencer/src/app/test_utils.rs +++ b/crates/astria-sequencer/src/app/test_utils.rs @@ -30,7 +30,7 @@ use crate::{ pub(crate) fn address_from_hex_string(s: &str) -> Address { let bytes = hex::decode(s).unwrap(); let arr: [u8; ADDRESS_LEN] = bytes.try_into().unwrap(); - Address::from_array(arr) + crate::astria_address(arr) } pub(crate) const ALICE_ADDRESS: &str = "1c0c490f1b5528d8173c5de46d131160e4b2c0c3"; @@ -47,7 +47,7 @@ pub(crate) fn get_alice_signing_key_and_address() -> (SigningKey, Address) { .try_into() .unwrap(); let alice_signing_key = SigningKey::from(alice_secret_bytes); - let alice = *alice_signing_key.verification_key().address(); + let alice = crate::astria_address(alice_signing_key.verification_key().address_bytes()); (alice_signing_key, alice) } @@ -58,7 +58,7 @@ pub(crate) fn get_bridge_signing_key_and_address() -> (SigningKey, Address) { .try_into() .unwrap(); let bridge_signing_key = SigningKey::from(bridge_secret_bytes); - let bridge = *bridge_signing_key.verification_key().address(); + let bridge = crate::astria_address(bridge_signing_key.verification_key().address_bytes()); (bridge_signing_key, bridge) } diff --git a/crates/astria-sequencer/src/app/tests_app.rs b/crates/astria-sequencer/src/app/tests_app.rs index 9e718df325..da7b266943 100644 --- a/crates/astria-sequencer/src/app/tests_app.rs +++ b/crates/astria-sequencer/src/app/tests_app.rs @@ -3,7 +3,6 @@ use std::collections::HashMap; use astria_core::{ primitive::v1::{ asset::DEFAULT_NATIVE_ASSET_DENOM, - Address, RollupId, }, protocol::transaction::v1alpha1::{ @@ -300,7 +299,7 @@ async fn app_create_sequencer_block_with_sequenced_data_and_deposits() { let (alice_signing_key, _) = get_alice_signing_key_and_address(); let (mut app, storage) = initialize_app_with_storage(None, vec![]).await; - let bridge_address = Address::from([99; 20]); + let bridge_address = crate::astria_address([99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); let asset_id = get_native_asset().id(); @@ -390,7 +389,7 @@ async fn app_execution_results_match_proposal_vs_after_proposal() { let (alice_signing_key, _) = get_alice_signing_key_and_address(); let (mut app, storage) = initialize_app_with_storage(None, vec![]).await; - let bridge_address = Address::from([99; 20]); + let bridge_address = crate::astria_address([99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); let asset_id = get_native_asset().id(); @@ -698,7 +697,7 @@ async fn app_end_block_validator_updates() { ]; let mut app = initialize_app(None, initial_validator_set).await; - let proposer_address = Address::try_from_slice([0u8; 20].as_ref()).unwrap(); + let proposer_address = crate::astria_address([0u8; 20]); let validator_updates = vec![ validator::Update { diff --git a/crates/astria-sequencer/src/app/tests_breaking_changes.rs b/crates/astria-sequencer/src/app/tests_breaking_changes.rs index cccf427a2b..54630124f4 100644 --- a/crates/astria-sequencer/src/app/tests_breaking_changes.rs +++ b/crates/astria-sequencer/src/app/tests_breaking_changes.rs @@ -17,7 +17,6 @@ use std::{ use astria_core::{ primitive::v1::{ asset::DEFAULT_NATIVE_ASSET_DENOM, - Address, RollupId, }, protocol::transaction::v1alpha1::{ @@ -74,7 +73,7 @@ async fn app_finalize_block_snapshot() { let (alice_signing_key, _) = get_alice_signing_key_and_address(); let (mut app, storage) = initialize_app_with_storage(None, vec![]).await; - let bridge_address = Address::from([99; 20]); + let bridge_address = crate::astria_address([99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); let asset_id = get_native_asset().id(); diff --git a/crates/astria-sequencer/src/app/tests_execute_transaction.rs b/crates/astria-sequencer/src/app/tests_execute_transaction.rs index 7ea28736c9..f2be1ffd24 100644 --- a/crates/astria-sequencer/src/app/tests_execute_transaction.rs +++ b/crates/astria-sequencer/src/app/tests_execute_transaction.rs @@ -5,7 +5,6 @@ use astria_core::{ primitive::v1::{ asset, asset::DEFAULT_NATIVE_ASSET_DENOM, - Address, RollupId, }, protocol::transaction::v1alpha1::{ @@ -378,7 +377,7 @@ async fn app_execute_transaction_ibc_relayer_change_invalid() { let genesis_state = GenesisState { accounts: default_genesis_accounts(), authority_sudo_address: alice_address, - ibc_sudo_address: Address::from([0; 20]), + ibc_sudo_address: crate::astria_address([0; 20]), ibc_relayer_addresses: vec![alice_address], native_asset_base_denomination: DEFAULT_NATIVE_ASSET_DENOM.to_string(), allowed_fee_assets: vec![DEFAULT_NATIVE_ASSET_DENOM.to_owned().into()], @@ -443,7 +442,7 @@ async fn app_execute_transaction_sudo_address_change_error() { let genesis_state = GenesisState { accounts: default_genesis_accounts(), authority_sudo_address: sudo_address, - ibc_sudo_address: [0u8; 20].into(), + ibc_sudo_address: crate::astria_address([0u8; 20]), ibc_relayer_addresses: vec![], native_asset_base_denomination: DEFAULT_NATIVE_ASSET_DENOM.to_string(), ibc_params: IBCParameters::default(), @@ -697,7 +696,7 @@ async fn app_execute_transaction_bridge_lock_action_ok() { let (alice_signing_key, alice_address) = get_alice_signing_key_and_address(); let mut app = initialize_app(None, vec![]).await; - let bridge_address = Address::from([99; 20]); + let bridge_address = crate::astria_address([99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); let asset_id = get_native_asset().id(); @@ -783,7 +782,7 @@ async fn app_execute_transaction_bridge_lock_action_invalid_for_eoa() { let mut app = initialize_app(None, vec![]).await; // don't actually register this address as a bridge address - let bridge_address = Address::from([99; 20]); + let bridge_address = crate::astria_address([99; 20]); let asset_id = get_native_asset().id(); let amount = 100; @@ -910,7 +909,7 @@ async fn app_stateful_check_fails_insufficient_total_balance() { // create a new key; will have 0 balance let keypair = SigningKey::new(OsRng); - let keypair_address = *keypair.verification_key().address(); + let keypair_address = crate::astria_address(keypair.verification_key().address_bytes()); // figure out needed fee for a single transfer let data = b"hello world".to_vec(); diff --git a/crates/astria-sequencer/src/authority/action.rs b/crates/astria-sequencer/src/authority/action.rs index 6c5386e0b4..d70fa3ce76 100644 --- a/crates/astria-sequencer/src/authority/action.rs +++ b/crates/astria-sequencer/src/authority/action.rs @@ -191,7 +191,7 @@ mod test { }; fee_change - .execute(&mut state, Address::from([1; 20])) + .execute(&mut state, crate::astria_address([1; 20])) .await .unwrap(); assert_eq!(state.get_transfer_base_fee().await.unwrap(), 10); @@ -205,7 +205,7 @@ mod test { }; fee_change - .execute(&mut state, Address::from([1; 20])) + .execute(&mut state, crate::astria_address([1; 20])) .await .unwrap(); assert_eq!(state.get_sequence_action_base_fee().await.unwrap(), 3); @@ -219,7 +219,7 @@ mod test { }; fee_change - .execute(&mut state, Address::from([1; 20])) + .execute(&mut state, crate::astria_address([1; 20])) .await .unwrap(); assert_eq!( @@ -239,7 +239,7 @@ mod test { }; fee_change - .execute(&mut state, Address::from([1; 20])) + .execute(&mut state, crate::astria_address([1; 20])) .await .unwrap(); assert_eq!(state.get_init_bridge_account_base_fee().await.unwrap(), 2); @@ -253,7 +253,7 @@ mod test { }; fee_change - .execute(&mut state, Address::from([1; 20])) + .execute(&mut state, crate::astria_address([1; 20])) .await .unwrap(); assert_eq!( @@ -272,7 +272,7 @@ mod test { }; fee_change - .execute(&mut state, Address::from([1; 20])) + .execute(&mut state, crate::astria_address([1; 20])) .await .unwrap(); assert_eq!(state.get_ics20_withdrawal_base_fee().await.unwrap(), 2); diff --git a/crates/astria-sequencer/src/authority/state_ext.rs b/crates/astria-sequencer/src/authority/state_ext.rs index c1da439891..35aceb3c5b 100644 --- a/crates/astria-sequencer/src/authority/state_ext.rs +++ b/crates/astria-sequencer/src/authority/state_ext.rs @@ -98,9 +98,9 @@ pub(crate) trait StateReadExt: StateRead { // return error because sudo key must be set bail!("sudo key not found"); }; - let SudoAddress(address) = + let SudoAddress(address_bytes) = SudoAddress::try_from_slice(&bytes).context("invalid sudo key bytes")?; - Ok(Address::from(address)) + Ok(crate::astria_address(address_bytes)) } #[instrument(skip(self))] @@ -144,7 +144,7 @@ pub(crate) trait StateWriteExt: StateWrite { fn put_sudo_address(&mut self, address: Address) -> Result<()> { self.put_raw( SUDO_STORAGE_KEY.to_string(), - borsh::to_vec(&SudoAddress(address.get())) + borsh::to_vec(&SudoAddress(address.bytes())) .context("failed to convert sudo address to vec")?, ); Ok(()) @@ -179,7 +179,6 @@ impl StateWriteExt for T {} #[cfg(test)] mod test { - use astria_core::primitive::v1::Address; use cnidarium::StateDelta; use tendermint::{ validator, @@ -206,7 +205,7 @@ mod test { .expect_err("no sudo address should exist at first"); // can write new - let mut address_expected = Address::try_from_slice(&[42u8; 20]).unwrap(); + let mut address_expected = crate::astria_address([42u8; 20]); state .put_sudo_address(address_expected) .expect("writing sudo address should not fail"); @@ -220,7 +219,7 @@ mod test { ); // can rewrite with new value - address_expected = Address::try_from_slice(&[41u8; 20]).unwrap(); + address_expected = crate::astria_address([41u8; 20]); state .put_sudo_address(address_expected) .expect("writing sudo address should not fail"); diff --git a/crates/astria-sequencer/src/bridge/bridge_lock_action.rs b/crates/astria-sequencer/src/bridge/bridge_lock_action.rs index dd356054b7..62330d93cd 100644 --- a/crates/astria-sequencer/src/bridge/bridge_lock_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_lock_action.rs @@ -167,7 +167,7 @@ mod test { state.put_transfer_base_fee(transfer_fee).unwrap(); state.put_bridge_lock_byte_cost_multiplier(2); - let bridge_address = Address::from([1; 20]); + let bridge_address = crate::astria_address([1; 20]); let asset_id = asset::Id::from_denom("test"); let bridge_lock = BridgeLockAction { to: bridge_address, @@ -184,7 +184,7 @@ mod test { .unwrap(); state.put_allowed_fee_asset(asset_id); - let from_address = Address::from([2; 20]); + let from_address = crate::astria_address([2; 20]); // not enough balance; should fail state @@ -226,7 +226,7 @@ mod test { state.put_transfer_base_fee(transfer_fee).unwrap(); state.put_bridge_lock_byte_cost_multiplier(2); - let bridge_address = Address::from([1; 20]); + let bridge_address = crate::astria_address([1; 20]); let asset_id = asset::Id::from_denom("test"); let bridge_lock = BridgeLockAction { to: bridge_address, @@ -243,7 +243,7 @@ mod test { .unwrap(); state.put_allowed_fee_asset(asset_id); - let from_address = Address::from([2; 20]); + let from_address = crate::astria_address([2; 20]); // not enough balance; should fail state diff --git a/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs b/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs index ba6abcdfad..076d775629 100644 --- a/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs +++ b/crates/astria-sequencer/src/bridge/bridge_unlock_action.rs @@ -94,8 +94,8 @@ mod test { let asset_id = asset::Id::from_denom("test"); let transfer_amount = 100; - let address = Address::from([1; 20]); - let to_address = Address::from([2; 20]); + let address = crate::astria_address([1; 20]); + let to_address = crate::astria_address([2; 20]); let bridge_unlock = BridgeUnlockAction { to: to_address, @@ -126,8 +126,8 @@ mod test { let transfer_amount = 100; state.put_transfer_base_fee(transfer_fee).unwrap(); - let bridge_address = Address::from([1; 20]); - let to_address = Address::from([2; 20]); + let bridge_address = crate::astria_address([1; 20]); + let to_address = crate::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); @@ -177,8 +177,8 @@ mod test { let transfer_amount = 100; state.put_transfer_base_fee(transfer_fee).unwrap(); - let bridge_address = Address::from([1; 20]); - let to_address = Address::from([2; 20]); + let bridge_address = crate::astria_address([1; 20]); + let to_address = crate::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); diff --git a/crates/astria-sequencer/src/bridge/state_ext.rs b/crates/astria-sequencer/src/bridge/state_ext.rs index b303733379..e63b3c232d 100644 --- a/crates/astria-sequencer/src/bridge/state_ext.rs +++ b/crates/astria-sequencer/src/bridge/state_ext.rs @@ -301,7 +301,6 @@ mod test { use astria_core::{ primitive::v1::{ asset::Id, - Address, RollupId, }, sequencerblock::v1alpha1::block::Deposit, @@ -319,7 +318,7 @@ mod test { let snapshot = storage.latest_snapshot(); let state = StateDelta::new(snapshot); - let address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let address = crate::astria_address([42u8; 20]); // uninitialized ok assert_eq!( @@ -338,7 +337,7 @@ mod test { let mut state = StateDelta::new(snapshot); let mut rollup_id = RollupId::new([1u8; 32]); - let address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let address = crate::astria_address([42u8; 20]); // can write new state.put_bridge_account_rollup_id(&address, &rollup_id); @@ -367,7 +366,7 @@ mod test { // can write additional account and both valid let rollup_id_1 = RollupId::new([2u8; 32]); - let address_1 = Address::try_from_slice(&[41u8; 20]).unwrap(); + let address_1 = crate::astria_address([41u8; 20]); state.put_bridge_account_rollup_id(&address_1, &rollup_id_1); assert_eq!( state @@ -396,7 +395,7 @@ mod test { let snapshot = storage.latest_snapshot(); let state = StateDelta::new(snapshot); - let address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let address = crate::astria_address([42u8; 20]); state .get_bridge_account_asset_id(&address) .await @@ -409,7 +408,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state = StateDelta::new(snapshot); - let address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let address = crate::astria_address([42u8; 20]); let mut asset = Id::from_denom("asset_0"); // can write @@ -440,7 +439,7 @@ mod test { ); // writing to other account also ok - let address_1 = Address::try_from_slice(&[41u8; 20]).unwrap(); + let address_1 = crate::astria_address([41u8; 20]); let asset_1 = Id::from_denom("asset_0"); state .put_bridge_account_asset_id(&address_1, &asset_1) @@ -563,7 +562,7 @@ mod test { let mut state = StateDelta::new(snapshot); let rollup_id = RollupId::new([1u8; 32]); - let bridge_address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let bridge_address = crate::astria_address([42u8; 20]); let mut amount = 10u128; let asset = Id::from_denom("asset_0"); let destination_chain_address = "0xdeadbeef"; @@ -675,7 +674,7 @@ mod test { let mut state = StateDelta::new(snapshot); let rollup_id_0 = RollupId::new([1u8; 32]); - let bridge_address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let bridge_address = crate::astria_address([42u8; 20]); let amount = 10u128; let asset = Id::from_denom("asset_0"); let destination_chain_address = "0xdeadbeef"; @@ -746,7 +745,7 @@ mod test { let mut state = StateDelta::new(snapshot); let rollup_id = RollupId::new([1u8; 32]); - let bridge_address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let bridge_address = crate::astria_address([42u8; 20]); let amount = 10u128; let asset = Id::from_denom("asset_0"); let destination_chain_address = "0xdeadbeef"; @@ -801,7 +800,7 @@ mod test { let mut state = StateDelta::new(snapshot); let rollup_id = RollupId::new([1u8; 32]); - let bridge_address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let bridge_address = crate::astria_address([42u8; 20]); let amount = 10u128; let asset = Id::from_denom("asset_0"); let destination_chain_address = "0xdeadbeef"; @@ -893,7 +892,7 @@ mod test { let mut state = StateDelta::new(snapshot); let rollup_id = RollupId::new([1u8; 32]); - let bridge_address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let bridge_address = crate::astria_address([42u8; 20]); let amount = 10u128; let asset = Id::from_denom("asset_0"); let destination_chain_address = "0xdeadbeef"; diff --git a/crates/astria-sequencer/src/config.rs b/crates/astria-sequencer/src/config.rs index 234a67ca5e..edcbb17e91 100644 --- a/crates/astria-sequencer/src/config.rs +++ b/crates/astria-sequencer/src/config.rs @@ -5,6 +5,8 @@ use serde::{ Serialize, }; +pub(crate) const ADDRESS_PREFIX: &str = "astria"; + // Allowed `struct_excessive_bools` because this is used as a container // for deserialization. Making this a builder-pattern is not actionable. #[allow(clippy::struct_excessive_bools)] diff --git a/crates/astria-sequencer/src/genesis.rs b/crates/astria-sequencer/src/genesis.rs index f6972f8412..064140f424 100644 --- a/crates/astria-sequencer/src/genesis.rs +++ b/crates/astria-sequencer/src/genesis.rs @@ -48,8 +48,8 @@ where { use serde::de::Error as _; let bytes: Vec = hex::serde::deserialize(deserializer)?; - Address::try_from_slice(&bytes) - .map_err(|e| D::Error::custom(format!("failed constructing address from bytes: {e}"))) + crate::try_astria_address(&bytes) + .map_err(|e| D::Error::custom(format!("failed constructing address from bytes: {e:?}"))) } fn deserialize_addresses<'de, D>(deserializer: D) -> Result, D::Error> @@ -68,8 +68,8 @@ where let s = s.as_str().ok_or(D::Error::custom("expected string"))?; let bytes: Vec = hex::decode(s) .map_err(|e| D::Error::custom(format!("failed decoding hex string: {e}")))?; - Address::try_from_slice(&bytes).map_err(|e| { - D::Error::custom(format!("failed constructing address from bytes: {e}")) + crate::try_astria_address(&bytes).map_err(|e| { + D::Error::custom(format!("failed constructing address from bytes: {e:?}")) }) }) .collect() diff --git a/crates/astria-sequencer/src/ibc/ics20_transfer.rs b/crates/astria-sequencer/src/ibc/ics20_transfer.rs index a477a8e236..a96fc40bf7 100644 --- a/crates/astria-sequencer/src/ibc/ics20_transfer.rs +++ b/crates/astria-sequencer/src/ibc/ics20_transfer.rs @@ -440,7 +440,7 @@ async fn execute_ics20_transfer( packet_data.receiver }; - let recipient = Address::try_from_slice( + let recipient = crate::try_astria_address( &hex::decode(recipient).context("failed to decode recipient as hex string")?, ) .context("invalid recipient address")?; @@ -655,7 +655,7 @@ mod test { .await .expect("valid ics20 transfer to user account; recipient, memo, and asset ID are valid"); - let recipient = Address::try_from_slice( + let recipient = crate::try_astria_address( &hex::decode("1c0c490f1b5528d8173c5de46d131160e4b2c0c3").unwrap(), ) .unwrap(); @@ -676,7 +676,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot.clone()); - let bridge_address = Address::from([99; 20]); + let bridge_address = crate::astria_address([99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); let denom: Denom = "dest_port/dest_channel/nootasset".to_string().into(); @@ -729,7 +729,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot.clone()); - let bridge_address = Address::from([99; 20]); + let bridge_address = crate::astria_address([99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); let denom: Denom = "dest_port/dest_channel/nootasset".to_string().into(); @@ -767,7 +767,7 @@ mod test { let snapshot = storage.latest_snapshot(); let mut state_tx = StateDelta::new(snapshot.clone()); - let bridge_address = Address::from([99; 20]); + let bridge_address = crate::astria_address([99; 20]); let rollup_id = RollupId::from_unhashed_bytes(b"testchainid"); let denom: Denom = "dest_port/dest_channel/nootasset".to_string().into(); @@ -837,7 +837,7 @@ mod test { .await .expect("valid ics20 transfer to user account; recipient, memo, and asset ID are valid"); - let recipient = Address::try_from_slice(&hex::decode(address_string).unwrap()).unwrap(); + let recipient = crate::try_astria_address(&hex::decode(address_string).unwrap()).unwrap(); let balance = state_tx .get_account_balance(recipient, base_denom.id()) .await @@ -891,7 +891,7 @@ mod test { .await .expect("valid ics20 refund to user account; recipient, memo, and asset ID are valid"); - let recipient = Address::try_from_slice(&hex::decode(address_string).unwrap()).unwrap(); + let recipient = crate::try_astria_address(&hex::decode(address_string).unwrap()).unwrap(); let balance = state_tx .get_account_balance(recipient, base_denom.id()) .await diff --git a/crates/astria-sequencer/src/ibc/state_ext.rs b/crates/astria-sequencer/src/ibc/state_ext.rs index 5e02e0193c..9a417d5055 100644 --- a/crates/astria-sequencer/src/ibc/state_ext.rs +++ b/crates/astria-sequencer/src/ibc/state_ext.rs @@ -76,9 +76,9 @@ pub(crate) trait StateReadExt: StateRead { // ibc sudo key must be set bail!("ibc sudo key not found"); }; - let SudoAddress(address) = + let SudoAddress(address_bytes) = SudoAddress::try_from_slice(&bytes).context("invalid ibc sudo key bytes")?; - Ok(Address::from(address)) + Ok(crate::astria_address(address_bytes)) } #[instrument(skip(self))] @@ -124,7 +124,7 @@ pub(crate) trait StateWriteExt: StateWrite { fn put_ibc_sudo_address(&mut self, address: Address) -> Result<()> { self.put_raw( IBC_SUDO_STORAGE_KEY.to_string(), - borsh::to_vec(&SudoAddress(address.get())) + borsh::to_vec(&SudoAddress(address.bytes())) .context("failed to convert sudo address to vec")?, ); Ok(()) @@ -154,10 +154,7 @@ impl StateWriteExt for T {} #[cfg(test)] mod test { - use astria_core::primitive::v1::{ - asset::Id, - Address, - }; + use astria_core::primitive::v1::asset::Id; use cnidarium::StateDelta; use ibc_types::core::channel::ChannelId; @@ -186,7 +183,7 @@ mod test { let mut state = StateDelta::new(snapshot); // can write new - let mut address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let mut address = crate::astria_address([42u8; 20]); state .put_ibc_sudo_address(address) .expect("writing sudo address should not fail"); @@ -200,7 +197,7 @@ mod test { ); // can rewrite with new value - address = Address::try_from_slice(&[41u8; 20]).unwrap(); + address = crate::astria_address([41u8; 20]); state .put_ibc_sudo_address(address) .expect("writing sudo address should not fail"); @@ -221,7 +218,7 @@ mod test { let state = StateDelta::new(snapshot); // unset address returns false - let address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let address = crate::astria_address([42u8; 20]); assert!( !state .is_ibc_relayer(&address) @@ -238,7 +235,7 @@ mod test { let mut state = StateDelta::new(snapshot); // can write - let address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let address = crate::astria_address([42u8; 20]); state.put_ibc_relayer_address(&address); assert!( state @@ -266,7 +263,7 @@ mod test { let mut state = StateDelta::new(snapshot); // can write - let address = Address::try_from_slice(&[42u8; 20]).unwrap(); + let address = crate::astria_address([42u8; 20]); state.put_ibc_relayer_address(&address); assert!( state @@ -277,7 +274,7 @@ mod test { ); // can write multiple - let address_1 = Address::try_from_slice(&[41u8; 20]).unwrap(); + let address_1 = crate::astria_address([41u8; 20]); state.put_ibc_relayer_address(&address_1); assert!( state diff --git a/crates/astria-sequencer/src/lib.rs b/crates/astria-sequencer/src/lib.rs index 96e0077613..443576febc 100644 --- a/crates/astria-sequencer/src/lib.rs +++ b/crates/astria-sequencer/src/lib.rs @@ -21,7 +21,31 @@ pub(crate) mod state_ext; pub(crate) mod transaction; mod utils; +use astria_core::primitive::v1::{ + Address, + AddressError, +}; pub use build_info::BUILD_INFO; pub use config::Config; +pub(crate) use config::ADDRESS_PREFIX; pub use sequencer::Sequencer; pub use telemetry; + +/// Constructs an [`Address`] prefixed by `"astria"`. +pub(crate) fn astria_address(array: [u8; astria_core::primitive::v1::ADDRESS_LEN]) -> Address { + Address::builder() + .array(array) + .prefix(ADDRESS_PREFIX) + .build() +} + +/// Tries to construct an [`Address`] prefixed by `"astria"` from a byte slice. +/// +/// # Errors +/// Fails if the slice does not contain 20 bytes. +pub(crate) fn try_astria_address(slice: &[u8]) -> Result { + Address::builder() + .slice(slice) + .prefix(ADDRESS_PREFIX) + .try_build() +} diff --git a/crates/astria-sequencer/src/mempool.rs b/crates/astria-sequencer/src/mempool.rs index 5cd5addc6e..578895784b 100644 --- a/crates/astria-sequencer/src/mempool.rs +++ b/crates/astria-sequencer/src/mempool.rs @@ -62,13 +62,16 @@ impl PartialOrd for TransactionPriority { pub(crate) struct EnqueuedTransaction { tx_hash: [u8; 32], signed_tx: Arc, + address: Address, } impl EnqueuedTransaction { fn new(signed_tx: SignedTransaction) -> Self { + let address = crate::astria_address(signed_tx.verification_key().address_bytes()); Self { tx_hash: signed_tx.sha256_of_proto_encoding(), signed_tx: Arc::new(signed_tx), + address, } } @@ -94,7 +97,7 @@ impl EnqueuedTransaction { } pub(crate) fn address(&self) -> &Address { - self.signed_tx.verification_key().address() + &self.address } } @@ -175,9 +178,11 @@ impl Mempool { /// removes a transaction from the mempool pub(crate) async fn remove(&self, tx_hash: [u8; 32]) { + let (signed_tx, address) = dummy_signed_tx(); let enqueued_tx = EnqueuedTransaction { tx_hash, - signed_tx: dummy_signed_tx().clone(), + signed_tx, + address, }; self.inner.write().await.remove(&enqueued_tx); } @@ -253,21 +258,23 @@ impl Mempool { /// this `signed_tx` field is ignored in the `PartialEq` and `Hash` impls of `EnqueuedTransaction` - /// only the tx hash is considered. So we create an `EnqueuedTransaction` on the fly with the /// correct tx hash and this dummy signed tx when removing from the queue. -fn dummy_signed_tx() -> &'static Arc { - static TX: OnceLock> = OnceLock::new(); - TX.get_or_init(|| { +fn dummy_signed_tx() -> (Arc, Address) { + static TX: OnceLock<(Arc, Address)> = OnceLock::new(); + let (signed_tx, address) = TX.get_or_init(|| { let actions = vec![]; let params = TransactionParams { nonce: 0, chain_id: String::new(), }; let signing_key = SigningKey::from([0; 32]); + let address = crate::astria_address(signing_key.verification_key().address_bytes()); let unsigned_tx = UnsignedTransaction { actions, params, }; - Arc::new(unsigned_tx.into_signed(&signing_key)) - }) + (Arc::new(unsigned_tx.into_signed(&signing_key)), address) + }); + (signed_tx.clone(), *address) } #[cfg(test)] @@ -346,14 +353,17 @@ mod test { let tx0 = EnqueuedTransaction { tx_hash: [0; 32], signed_tx: Arc::new(get_mock_tx(0)), + address: crate::astria_address(get_mock_tx(0).verification_key().address_bytes()), }; let other_tx0 = EnqueuedTransaction { tx_hash: [0; 32], signed_tx: Arc::new(get_mock_tx(1)), + address: crate::astria_address(get_mock_tx(1).verification_key().address_bytes()), }; let tx1 = EnqueuedTransaction { tx_hash: [1; 32], signed_tx: Arc::new(get_mock_tx(0)), + address: crate::astria_address(get_mock_tx(0).verification_key().address_bytes()), }; assert!(tx0 == other_tx0); assert!(tx0 != tx1); @@ -467,7 +477,8 @@ mod test { let (alice_signing_key, alice_address) = crate::app::test_utils::get_alice_signing_key_and_address(); - let other_address = *other_signing_key.verification_key().address(); + let other_address = + crate::astria_address(other_signing_key.verification_key().address_bytes()); // Create a getter fn which will returns 1 for alice's current account nonce, and 101 for // the other signer's. @@ -538,15 +549,23 @@ mod test { // Check the pending nonce for alice is 1 and for the other signer is 101. let alice_address = crate::app::test_utils::get_alice_signing_key_and_address().1; assert_eq!(mempool.pending_nonce(&alice_address).await.unwrap(), 1); - let other_address = *other_signing_key.verification_key().address(); + let other_address = + crate::astria_address(other_signing_key.verification_key().address_bytes()); assert_eq!(mempool.pending_nonce(&other_address).await.unwrap(), 101); // Check the pending nonce for an address with no enqueued txs is `None`. assert!( mempool - .pending_nonce(&Address::from([1; 20])) + .pending_nonce(&crate::astria_address([1; 20])) .await .is_none() ); } + + #[test] + fn enqueued_transaction_can_be_instantiated() { + // This just tests that the constructor does not fail. + let signed_tx = crate::app::test_utils::get_mock_tx(0); + let _ = EnqueuedTransaction::new(signed_tx); + } } diff --git a/crates/astria-sequencer/src/proposal/commitment.rs b/crates/astria-sequencer/src/proposal/commitment.rs index 65914b8492..5e5b0d820a 100644 --- a/crates/astria-sequencer/src/proposal/commitment.rs +++ b/crates/astria-sequencer/src/proposal/commitment.rs @@ -89,12 +89,9 @@ pub(crate) fn generate_rollup_datas_commitment( mod test { use astria_core::{ crypto::SigningKey, - primitive::v1::{ - asset::{ - Denom, - DEFAULT_NATIVE_ASSET_DENOM, - }, - Address, + primitive::v1::asset::{ + Denom, + DEFAULT_NATIVE_ASSET_DENOM, }, protocol::transaction::v1alpha1::{ action::{ @@ -123,7 +120,7 @@ mod test { fee_asset_id: get_native_asset().id(), }; let transfer_action = TransferAction { - to: Address::from([0u8; 20]), + to: crate::astria_address([0u8; 20]), amount: 1, asset_id: get_native_asset().id(), fee_asset_id: get_native_asset().id(), @@ -178,7 +175,7 @@ mod test { fee_asset_id: get_native_asset().id(), }; let transfer_action = TransferAction { - to: Address::from([0u8; 20]), + to: crate::astria_address([0u8; 20]), amount: 1, asset_id: get_native_asset().id(), fee_asset_id: get_native_asset().id(), diff --git a/crates/astria-sequencer/src/service/consensus.rs b/crates/astria-sequencer/src/service/consensus.rs index 993cf74d98..357bfa5f95 100644 --- a/crates/astria-sequencer/src/service/consensus.rs +++ b/crates/astria-sequencer/src/service/consensus.rs @@ -225,7 +225,6 @@ mod test { }, primitive::v1::{ asset::DEFAULT_NATIVE_ASSET_DENOM, - Address, RollupId, }, protocol::transaction::v1alpha1::{ @@ -461,8 +460,8 @@ mod test { fn default() -> Self { Self { accounts: vec![], - authority_sudo_address: Address::from([0; 20]), - ibc_sudo_address: Address::from([0; 20]), + authority_sudo_address: crate::astria_address([0; 20]), + ibc_sudo_address: crate::astria_address([0; 20]), ibc_relayer_addresses: vec![], native_asset_base_denomination: DEFAULT_NATIVE_ASSET_DENOM.to_string(), ibc_params: penumbra_ibc::params::IBCParameters::default(), @@ -475,7 +474,7 @@ mod test { async fn new_consensus_service(funded_key: Option) -> (Consensus, Mempool) { let accounts = if funded_key.is_some() { vec![crate::genesis::Account { - address: *funded_key.unwrap().address(), + address: crate::astria_address(funded_key.unwrap().address_bytes()), balance: 10u128.pow(19), }] } else { diff --git a/crates/astria-sequencer/src/service/info/mod.rs b/crates/astria-sequencer/src/service/info/mod.rs index 7433c14a0a..5329325e8d 100644 --- a/crates/astria-sequencer/src/service/info/mod.rs +++ b/crates/astria-sequencer/src/service/info/mod.rs @@ -148,12 +148,9 @@ impl Service for Info { #[cfg(test)] mod test { - use astria_core::primitive::v1::{ - asset::{ - Denom, - DEFAULT_NATIVE_ASSET_DENOM, - }, - Address, + use astria_core::primitive::v1::asset::{ + Denom, + DEFAULT_NATIVE_ASSET_DENOM, }; use cnidarium::StateDelta; use prost::Message as _; @@ -191,7 +188,7 @@ mod test { initialize_native_asset(DEFAULT_NATIVE_ASSET_DENOM); - let address = Address::try_from_slice( + let address = crate::try_astria_address( &hex::decode("a034c743bed8f26cb8ee7b8db2230fd8347ae131").unwrap(), ) .unwrap(); diff --git a/crates/astria-sequencer/src/service/mempool.rs b/crates/astria-sequencer/src/service/mempool.rs index b688bf376a..4cc1b4b029 100644 --- a/crates/astria-sequencer/src/service/mempool.rs +++ b/crates/astria-sequencer/src/service/mempool.rs @@ -189,7 +189,9 @@ async fn handle_check_tx( // tx is valid, push to mempool let current_account_nonce = state - .get_account_nonce(*signed_tx.verification_key().address()) + .get_account_nonce(crate::astria_address( + signed_tx.verification_key().address_bytes(), + )) .await .expect("can fetch account nonce"); diff --git a/crates/astria-sequencer/src/transaction/checks.rs b/crates/astria-sequencer/src/transaction/checks.rs index a2074e0e2b..89423cd248 100644 --- a/crates/astria-sequencer/src/transaction/checks.rs +++ b/crates/astria-sequencer/src/transaction/checks.rs @@ -31,7 +31,7 @@ pub(crate) async fn check_nonce_mempool( tx: &SignedTransaction, state: &S, ) -> anyhow::Result<()> { - let signer_address = *tx.verification_key().address(); + let signer_address = crate::astria_address(tx.verification_key().address_bytes()); let curr_nonce = state .get_account_nonce(signer_address) .await @@ -62,7 +62,7 @@ pub(crate) async fn check_balance_mempool( tx: &SignedTransaction, state: &S, ) -> anyhow::Result<()> { - let signer_address = *tx.verification_key().address(); + let signer_address = crate::astria_address(tx.verification_key().address_bytes()); check_balance_for_total_fees(tx.unsigned_transaction(), signer_address, state).await?; Ok(()) } @@ -335,7 +335,7 @@ mod test { asset_id: other_asset, amount, fee_asset_id: native_asset, - to: [0; ADDRESS_LEN].into(), + to: crate::astria_address([0; ADDRESS_LEN]), }), Action::Sequence(SequenceAction { rollup_id: RollupId::from_unhashed_bytes([0; 32]), @@ -397,7 +397,7 @@ mod test { asset_id: other_asset, amount, fee_asset_id: native_asset, - to: [0; ADDRESS_LEN].into(), + to: crate::astria_address([0; ADDRESS_LEN]), }), Action::Sequence(SequenceAction { rollup_id: RollupId::from_unhashed_bytes([0; 32]), diff --git a/crates/astria-sequencer/src/transaction/mod.rs b/crates/astria-sequencer/src/transaction/mod.rs index 1eaa5f2a30..096c62b694 100644 --- a/crates/astria-sequencer/src/transaction/mod.rs +++ b/crates/astria-sequencer/src/transaction/mod.rs @@ -47,7 +47,7 @@ pub(crate) async fn check_stateful( tx: &SignedTransaction, state: &S, ) -> anyhow::Result<()> { - let signer_address = *tx.verification_key().address(); + let signer_address = crate::astria_address(tx.verification_key().address_bytes()); tx.unsigned_transaction() .check_stateful(state, signer_address) .await @@ -57,7 +57,7 @@ pub(crate) async fn execute( tx: &SignedTransaction, state: &mut S, ) -> anyhow::Result<()> { - let signer_address = *tx.verification_key().address(); + let signer_address = crate::astria_address(tx.verification_key().address_bytes()); tx.unsigned_transaction() .execute(state, signer_address) .await diff --git a/proto/primitives/astria/primitive/v1/types.proto b/proto/primitives/astria/primitive/v1/types.proto index b86a1c9b70..76f47ba66a 100644 --- a/proto/primitives/astria/primitive/v1/types.proto +++ b/proto/primitives/astria/primitive/v1/types.proto @@ -47,7 +47,7 @@ message RollupId { // Astria addresses are derived from the ed25519 public key, // using the first 20 bytes of the sha256 hash. message Address { - bytes inner = 1; + bytes inner = 1 [deprecated = true]; string bech32m = 2; } diff --git a/proto/protocolapis/astria/protocol/transactions/v1alpha1/types.proto b/proto/protocolapis/astria/protocol/transactions/v1alpha1/types.proto index 17d462f2ff..34a6c1073c 100644 --- a/proto/protocolapis/astria/protocol/transactions/v1alpha1/types.proto +++ b/proto/protocolapis/astria/protocol/transactions/v1alpha1/types.proto @@ -108,7 +108,7 @@ message Ics20Withdrawal { string destination_chain_address = 3; // an Astria address to use to return funds from this withdrawal // in the case it fails. - bytes return_address = 4; + astria.primitive.v1.Address return_address = 4; // the height (on Astria) at which this transfer expires. IbcHeight timeout_height = 5; // the unix timestamp (in nanoseconds) at which this transfer expires. From cf5de1b04ddc8e74ace26bfe16d6aeff6730ebca Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Thu, 6 Jun 2024 19:25:11 +0200 Subject: [PATCH 07/15] rip out arrayvec, ensure that prefixes are valid hrp, make deprecated bytes and bech32m fields mutually exclusive --- Cargo.lock | 1 - crates/astria-core/Cargo.toml | 3 +- crates/astria-core/src/primitive/v1/mod.rs | 129 +++++++----------- ...core__primitive__v1__tests__snapshots.snap | 1 - ...alpha1__test__signed_transaction_hash.snap | 60 ++++---- 5 files changed, 83 insertions(+), 111 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1f21ad9fc2..019b42f03e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -667,7 +667,6 @@ dependencies = [ name = "astria-core" version = "0.1.0" dependencies = [ - "arrayvec 0.7.4", "astria-core", "astria-merkle", "base64 0.21.7", diff --git a/crates/astria-core/Cargo.toml b/crates/astria-core/Cargo.toml index 94ed689134..b9ca603ebe 100644 --- a/crates/astria-core/Cargo.toml +++ b/crates/astria-core/Cargo.toml @@ -16,6 +16,7 @@ keywords = ["astria", "grpc", "rpc", "blockchain", "execution", "protobuf"] # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] +bech32 = "0.11.0" brotli = { version = "5.0.0", optional = true } celestia-types = { version = "0.1.1", optional = true } pbjson = { version = "0.6.0", optional = true } @@ -44,8 +45,6 @@ tracing = { workspace = true } base64-serde = { workspace = true, optional = true } base64 = { workspace = true } zeroize = { version = "1.7.0", features = ["zeroize_derive"] } -bech32 = "0.11.0" -arrayvec = "0.7.4" [features] celestia = ["dep:celestia-types"] diff --git a/crates/astria-core/src/primitive/v1/mod.rs b/crates/astria-core/src/primitive/v1/mod.rs index f8da0726d9..3a560a7963 100644 --- a/crates/astria-core/src/primitive/v1/mod.rs +++ b/crates/astria-core/src/primitive/v1/mod.rs @@ -3,10 +3,7 @@ pub mod u128; use base64::{ display::Base64Display, - prelude::{ - Engine as _, - BASE64_STANDARD, - }, + prelude::BASE64_STANDARD, }; use bytes::Bytes; use sha2::{ @@ -21,12 +18,7 @@ use crate::{ pub const ADDRESS_LEN: usize = 20; /// The human readable prefix of astria addresses (also known as bech32 HRP). -pub const HUMAN_READABLE_ADDRESS_PREFIX: &str = "astria"; -// The compile-time generated bech32::Hrp to avoid redoing it on every encode. -// Intentionally kept crate-private to not make bech32 part of the crate API. -const BECH32_HRP: bech32::Hrp = bech32::Hrp::parse_unchecked(HUMAN_READABLE_ADDRESS_PREFIX); -// The length of astria addresses as bech32m strings. -const ADDRESS_BECH32M_LENGTH: usize = 45; +pub const ASTRIA_ADDRESS_PREFIX: &str = "astria"; pub const ROLLUP_ID_LEN: usize = 32; pub const FEE_ASSET_ID_LEN: usize = 32; @@ -266,17 +258,14 @@ impl AddressError { }) } - fn bech32_hrp_too_long(source: arrayvec::CapacityError) -> Self { - Self(AddressErrorKind::Bech32HrpTooLong { + fn invalid_prefix(source: bech32::primitives::hrp::Error) -> Self { + Self(AddressErrorKind::InvalidPrefix { source, }) } - fn fields_dont_match(bytes: Bytes, bech32m: [u8; ADDRESS_LEN]) -> Self { - Self(AddressErrorKind::FieldsDontMatch { - bytes, - bech32m, - }) + fn fields_are_mutually_exclusive() -> Self { + Self(AddressErrorKind::FieldsAreMutuallyExclusive) } fn incorrect_address_length(received: usize) -> Self { @@ -290,22 +279,14 @@ impl AddressError { enum AddressErrorKind { #[error("failed decoding provided bech32m string")] Bech32mDecode { source: bech32::DecodeError }, - #[error( - "address protobuf contained mismatched address in its `bytes` ({}) and `bech32m` ({}) fields", - BASE64_STANDARD.encode(.bytes), - BASE64_STANDARD.encode(.bech32m), - )] - FieldsDontMatch { - bytes: Bytes, - bech32m: [u8; ADDRESS_LEN], - }, + #[error("fields `inner` and `bech32m` are mutually exclusive, only one can be set")] + FieldsAreMutuallyExclusive, #[error("expected an address of 20 bytes, got `{received}`")] IncorrectAddressLength { received: usize }, - #[error( - "the bech32 human readable prefix exceeds the maximum permitted address capacity of 16 \ - bytes" - )] - Bech32HrpTooLong { source: arrayvec::CapacityError }, + #[error("the provided prefix was not a valid bech32 human readable prefix")] + InvalidPrefix { + source: bech32::primitives::hrp::Error, + }, } pub struct NoBytes; @@ -375,9 +356,7 @@ impl<'a, 'b> AddressBuilder, WithPrefix<'b>> { BytesInner::Slice(bytes) => <[u8; ADDRESS_LEN]>::try_from(bytes.as_ref()) .map_err(|_| AddressError::incorrect_address_length(bytes.len()))?, }; - let prefix = arrayvec::ArrayString::from(prefix.as_ref()) - .map_err(arrayvec::CapacityError::simplify) - .map_err(AddressError::bech32_hrp_too_long)?; + let prefix = bech32::Hrp::parse(&prefix).map_err(AddressError::invalid_prefix)?; Ok(Address { bytes, prefix, @@ -389,7 +368,7 @@ impl<'a, 'b> AddressBuilder, WithPrefix<'b>> { #[cfg_attr(feature = "serde", derive(serde::Serialize))] #[cfg_attr(feature = "serde", serde(into = "raw::Address"))] pub struct Address { - prefix: arrayvec::ArrayString<16>, + prefix: bech32::Hrp, bytes: [u8; ADDRESS_LEN], } @@ -427,16 +406,12 @@ impl Address { #[allow(clippy::missing_panics_doc)] #[must_use] pub fn to_raw(&self) -> raw::Address { - let mut bech32m = String::with_capacity(ADDRESS_BECH32M_LENGTH); - bech32::encode_lower_to_fmt::(&mut bech32m, BECH32_HRP, &self.bytes()) - .expect( - "must not fail because Address is tested to be ADDRESS_BECH32M_LENGTH long, which \ - is less than the permitted maximum bech32m checksum length", - ); + let bech32m = bech32::encode_lower::(self.prefix, &self.bytes()) + .expect("must 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 { - inner: Bytes::copy_from_slice(&self.bytes()), + inner: Bytes::new(), bech32m, } } @@ -467,14 +442,7 @@ impl Address { if inner.is_empty() { return Self::try_from_bech32m(bech32m); } - let address = Self::try_from_bech32m(bech32m)?; - if inner.as_ref() != &address.bytes { - return Err(AddressError::fields_dont_match( - inner.clone(), - address.bytes(), - )); - } - Ok(address) + Err(AddressError::fields_are_mutually_exclusive()) } } @@ -499,7 +467,7 @@ impl From
for raw::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, BECH32_HRP, &self.bytes()) { + match bech32::encode_lower_to_fmt::(f, self.prefix, &self.bytes()) { Ok(()) => Ok(()), Err(EncodeError::Fmt(err)) => Err(err), Err(err) => panic!( @@ -540,8 +508,8 @@ mod tests { AddressError, AddressErrorKind, ADDRESS_LEN, + ASTRIA_ADDRESS_PREFIX, }; - use crate::primitive::v1::BECH32_HRP; #[track_caller] fn assert_wrong_address_bytes(bad_account: &[u8]) { @@ -577,35 +545,31 @@ mod tests { } #[test] - fn const_astria_hrp_is_valid() { - let hrp = bech32::Hrp::parse(super::HUMAN_READABLE_ADDRESS_PREFIX).unwrap(); - assert_eq!(hrp, super::BECH32_HRP); - } - - #[test] - fn const_astria_bech32m_is_correct_length() { - let actual = bech32::encoded_length::( - super::BECH32_HRP, - &[42u8; super::ADDRESS_LEN], - ) - .unwrap(); - assert_eq!(super::ADDRESS_BECH32M_LENGTH, actual); + fn can_construct_protobuf_from_address_with_maximally_sized_prefix() { + // 83 is the maximal length of a hrp + let long_prefix = [b'a'; 83]; + let address = Address::builder() + .array([42u8; ADDRESS_LEN]) + .prefix(std::str::from_utf8(&long_prefix).unwrap()) + .build(); + let _ = address.into_raw(); } #[test] - fn mismatched_fields_in_protobuf_address_are_caught() { + fn bech32m_and_deprecated_bytes_field_are_mutually_exclusive() { // allow: `Address::inner` field is deprecated, but we must still check it #![allow(deprecated)] let bytes = Bytes::copy_from_slice(&[24u8; ADDRESS_LEN]); let bech32m = [42u8; ADDRESS_LEN]; let proto = super::raw::Address { inner: bytes.clone(), - bech32m: bech32::encode_lower::(super::BECH32_HRP, &bech32m).unwrap(), - }; - let expected = AddressErrorKind::FieldsDontMatch { - bytes, - bech32m, + bech32m: bech32::encode_lower::( + bech32::Hrp::parse(ASTRIA_ADDRESS_PREFIX).unwrap(), + &bech32m, + ) + .unwrap(), }; + let expected = AddressErrorKind::FieldsAreMutuallyExclusive; let actual = Address::try_from_raw(&proto) .expect_err("returned a valid address where it should have errored"); assert_eq!(expected, actual.0); @@ -621,33 +585,44 @@ mod tests { bech32m: String::new(), }; let address = Address::try_from_raw(&input).unwrap(); - assert_eq!("astria", &address.prefix); + assert_eq!("astria", address.prefix.as_str()); assert_eq!(bytes, address.bytes()); } #[test] - fn proto_with_missing_bytes_is_accepted_and_bytes_is_populated() { + fn proto_with_missing_bytes_is_accepted() { // allow: `Address::inner` field is deprecated, but we must still check it #![allow(deprecated)] let bytes = [42u8; ADDRESS_LEN]; let input = raw::Address { inner: Bytes::new(), - bech32m: bech32::encode_lower::(BECH32_HRP, &bytes).unwrap(), + bech32m: bech32::encode_lower::( + bech32::Hrp::parse(ASTRIA_ADDRESS_PREFIX).unwrap(), + &bytes, + ) + .unwrap(), }; let address = Address::try_from_raw(&input).unwrap(); assert_eq!(bytes, address.bytes()); } #[test] - fn protobuf_has_bytes_and_bech32m_populated() { + fn protobuf_only_has_bech32m_populated() { // allow: `Address::inner` field is deprecated, but we must still check it #![allow(deprecated)] let bytes = [42u8; ADDRESS_LEN]; let address = Address::builder().array(bytes).prefix("astria").build(); let output = address.into_raw(); - assert_eq!(&bytes[..], &*output.inner); + assert!( + output.inner.is_empty(), + "the deprecated bytes field must not be set" + ); assert_eq!( - bech32::encode_lower::(BECH32_HRP, &bytes).unwrap(), + bech32::encode_lower::( + bech32::Hrp::parse(ASTRIA_ADDRESS_PREFIX).unwrap(), + &bytes + ) + .unwrap(), output.bech32m ); } diff --git a/crates/astria-core/src/primitive/v1/snapshots/astria_core__primitive__v1__tests__snapshots.snap b/crates/astria-core/src/primitive/v1/snapshots/astria_core__primitive__v1__tests__snapshots.snap index 75cafa33cd..7aca827420 100644 --- a/crates/astria-core/src/primitive/v1/snapshots/astria_core__primitive__v1__tests__snapshots.snap +++ b/crates/astria-core/src/primitive/v1/snapshots/astria_core__primitive__v1__tests__snapshots.snap @@ -3,6 +3,5 @@ source: crates/astria-core/src/primitive/v1/mod.rs expression: address --- { - "inner": "KioqKioqKioqKioqKioqKioqKio=", "bech32m": "astria19g4z52329g4z52329g4z52329g4z5232ayenag" } diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/snapshots/astria_core__protocol__transaction__v1alpha1__test__signed_transaction_hash.snap b/crates/astria-core/src/protocol/transaction/v1alpha1/snapshots/astria_core__protocol__transaction__v1alpha1__test__signed_transaction_hash.snap index b2c38d455c..5a65b1ec82 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/snapshots/astria_core__protocol__transaction__v1alpha1__test__signed_transaction_hash.snap +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/snapshots/astria_core__protocol__transaction__v1alpha1__test__signed_transaction_hash.snap @@ -3,36 +3,36 @@ source: crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs expression: tx.sha256_of_proto_encoding() --- [ - 12, - 91, - 197, - 58, - 205, - 198, - 92, - 206, - 230, - 47, - 37, - 22, - 41, 34, - 190, - 29, - 87, - 44, - 191, + 82, + 88, + 30, + 41, + 64, + 178, + 49, + 45, + 195, + 239, + 10, + 174, + 39, + 237, + 81, + 217, + 210, + 116, + 141, + 114, + 137, 209, - 145, - 134, - 205, - 104, - 13, - 92, - 189, - 112, - 127, - 218, - 115, - 191 + 236, + 54, + 17, + 110, + 147, + 150, + 117, + 135, + 115 ] From e369919097a8f8d6a9654329eff380d84526d6ff Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Thu, 6 Jun 2024 19:31:43 +0200 Subject: [PATCH 08/15] add comments on protobuf address message --- .../astria-core/src/generated/astria.primitive.v1.rs | 12 ++++++++++-- proto/primitives/astria/primitive/v1/types.proto | 12 ++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/crates/astria-core/src/generated/astria.primitive.v1.rs b/crates/astria-core/src/generated/astria.primitive.v1.rs index 54dbeac75c..5fd85b6e0f 100644 --- a/crates/astria-core/src/generated/astria.primitive.v1.rs +++ b/crates/astria-core/src/generated/astria.primitive.v1.rs @@ -81,14 +81,22 @@ impl ::prost::Name for RollupId { } /// An Astria `Address`. /// -/// Astria addresses are derived from the ed25519 public key, -/// using the first 20 bytes of the sha256 hash. +/// Astria addresses are bech32m encoded strings, with the data part being the +/// first 20 entries of a sha256-hashed ed25519 public key. #[allow(clippy::derive_partial_eq_without_eq)] #[derive(Clone, PartialEq, ::prost::Message)] pub struct Address { + /// The first 20 bytes of a sha256-hashed ed25519 public key. + /// Implementors must avoid setting this field in favor of `bech32m`. + /// Implementors must not accept both `inner` and `bech32m` being set. + /// DEPRECATED: this field is deprecated and only exists for backward compatibility. + /// Astria services assume an implicit prefix of "astria" if this field is set. + /// Astria services will read this field but will never emit it. #[deprecated] #[prost(bytes = "bytes", tag = "1")] pub inner: ::prost::bytes::Bytes, + /// A bech32m encoded string. The data are the first 20 bytes of a sha256-hashed ed25519 + /// public key. Implementors must not accept both the `bytes` and `bech32m` being set. #[prost(string, tag = "2")] pub bech32m: ::prost::alloc::string::String, } diff --git a/proto/primitives/astria/primitive/v1/types.proto b/proto/primitives/astria/primitive/v1/types.proto index 76f47ba66a..e9fef40064 100644 --- a/proto/primitives/astria/primitive/v1/types.proto +++ b/proto/primitives/astria/primitive/v1/types.proto @@ -44,10 +44,18 @@ message RollupId { // An Astria `Address`. // -// Astria addresses are derived from the ed25519 public key, -// using the first 20 bytes of the sha256 hash. +// Astria addresses are bech32m encoded strings, with the data part being the +// first 20 entries of a sha256-hashed ed25519 public key. message Address { + // The first 20 bytes of a sha256-hashed ed25519 public key. + // Implementors must avoid setting this field in favor of `bech32m`. + // Implementors must not accept both `inner` and `bech32m` being set. + // DEPRECATED: this field is deprecated and only exists for backward compatibility. + // Astria services assume an implicit prefix of "astria" if this field is set. + // Astria services will read this field but will never emit it. bytes inner = 1 [deprecated = true]; + // A bech32m encoded string. The data are the first 20 bytes of a sha256-hashed ed25519 + // public key. Implementors must not accept both the `bytes` and `bech32m` being set. string bech32m = 2; } From 75bda6d13ecb26be4e4b8d3669d445938639cb10 Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Thu, 6 Jun 2024 19:50:09 +0200 Subject: [PATCH 09/15] fix after merge and update snapshots --- .../src/withdrawer/submitter/tests.rs | 16 ++++- .../astria-sequencer-client/src/tests/http.rs | 2 +- ...ransaction_with_every_action_snapshot.snap | 60 +++++++++---------- ..._changes__app_finalize_block_snapshot.snap | 60 +++++++++---------- 4 files changed, 74 insertions(+), 64 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/tests.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/tests.rs index 9f1f1fe053..cdfe07a34f 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/tests.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/tests.rs @@ -6,7 +6,11 @@ use std::{ use astria_core::{ generated::protocol::account::v1alpha1::NonceResponse, - primitive::v1::asset::Denom, + primitive::v1::{ + asset::Denom, + Address, + ASTRIA_ADDRESS_PREFIX, + }, protocol::transaction::v1alpha1::{ action::{ BridgeUnlockAction, @@ -167,7 +171,10 @@ fn make_ics20_withdrawal_action() -> Action { let inner = Ics20Withdrawal { denom: denom.clone(), destination_chain_address, - return_address: [0u8; 20].into(), + return_address: Address::builder() + .array([0u8; 20]) + .prefix(ASTRIA_ADDRESS_PREFIX) + .build(), amount: 99, memo: serde_json::to_string(&Ics20WithdrawalMemo { memo: "hello".to_string(), @@ -187,7 +194,10 @@ fn make_ics20_withdrawal_action() -> Action { fn make_bridge_unlock_action() -> Action { let denom = Denom::from("nria".to_string()); let inner = BridgeUnlockAction { - to: [0u8; 20].into(), + to: Address::builder() + .array([0u8; 20]) + .prefix(ASTRIA_ADDRESS_PREFIX) + .build(), amount: 99, memo: serde_json::to_vec(&BridgeUnlockMemo { block_number: 1.into(), diff --git a/crates/astria-sequencer-client/src/tests/http.rs b/crates/astria-sequencer-client/src/tests/http.rs index ccd463f63e..2234974112 100644 --- a/crates/astria-sequencer-client/src/tests/http.rs +++ b/crates/astria-sequencer-client/src/tests/http.rs @@ -269,7 +269,7 @@ async fn get_bridge_account_last_transaction_hash() { .await; let actual_response = client - .get_bridge_account_last_transaction_hash(ALICE_ADDRESS) + .get_bridge_account_last_transaction_hash(alice_address()) .await .unwrap() .into_raw(); 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 045758c049..22384b1ee9 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() --- [ - 232, - 213, - 217, - 84, - 137, - 197, - 192, + 98, + 92, + 234, + 52, + 36, + 227, + 123, + 177, + 50, + 149, + 185, + 20, + 226, + 143, + 234, 255, - 247, - 176, - 2, - 44, - 162, - 5, - 17, - 77, - 182, - 63, - 115, - 118, - 162, - 142, - 194, - 201, - 33, - 57, + 184, + 222, + 123, + 23, + 23, + 173, 178, - 49, - 190, - 13, - 110, - 227 + 200, + 225, + 125, + 222, + 140, + 120, + 187, + 188, + 65 ] 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 11207d1607..69b08aa1d1 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() --- [ - 247, - 141, - 59, - 2, - 241, - 28, - 238, - 183, - 204, - 108, - 89, - 90, - 223, - 104, + 219, + 243, + 219, + 31, + 187, 190, - 38, - 228, - 148, - 207, - 177, - 52, - 34, - 126, - 52, - 50, - 206, - 92, - 48, - 196, + 97, + 198, + 149, + 6, + 242, + 31, + 108, + 0, + 130, + 193, + 167, + 251, + 111, 245, - 153, - 30 + 62, + 243, + 252, + 170, + 209, + 246, + 243, + 161, + 231, + 196, + 145, + 64 ] From ff3f68dce180fb3f319f59604cf99c4503f1c983 Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Thu, 6 Jun 2024 20:17:45 +0200 Subject: [PATCH 10/15] use astria address prefix constant everywhere --- .../src/withdrawer/ethereum/convert.rs | 23 ++++++++++---- .../src/withdrawer/submitter/signer.rs | 11 +++++-- .../src/withdrawer/submitter/tests.rs | 6 ++-- crates/astria-cli/src/lib.rs | 3 +- crates/astria-core/src/crypto.rs | 3 ++ crates/astria-core/src/primitive/v1/mod.rs | 30 +++++++++++++------ .../src/protocol/transaction/v1alpha1/mod.rs | 13 ++++++-- .../src/extension_trait.rs | 2 +- .../astria-sequencer-client/src/tests/http.rs | 11 ++++--- crates/astria-sequencer/src/lib.rs | 3 +- 10 files changed, 77 insertions(+), 28 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/convert.rs b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/convert.rs index fcfa976fec..d1cf0c795c 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/convert.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/convert.rs @@ -7,6 +7,7 @@ use astria_core::{ Denom, }, Address, + ASTRIA_ADDRESS_PREFIX, }, protocol::transaction::v1alpha1::{ action::{ @@ -99,7 +100,7 @@ fn event_to_bridge_unlock( let action = BridgeUnlockAction { to: Address::builder() .array(event.destination_chain_address.to_fixed_bytes().into()) - .prefix("astria") + .prefix(ASTRIA_ADDRESS_PREFIX) .try_build() .wrap_err("failed to construct destination address")?, amount: event @@ -156,7 +157,7 @@ fn event_to_ics20_withdrawal( // bytes, but this won't work otherwise. return_address: Address::builder() .array(sender) - .prefix("astria") + .prefix(ASTRIA_ADDRESS_PREFIX) .try_build() .wrap_err("failed to construct return address")?, amount: event @@ -213,7 +214,11 @@ mod tests { }; let expected_action = BridgeUnlockAction { - to: Address::builder().array([1u8; 20]).prefix("astria").build(), + to: Address::builder() + .array([1u8; 20]) + .prefix(ASTRIA_ADDRESS_PREFIX) + .try_build() + .unwrap(), amount: 99, memo: serde_json::to_vec(&BridgeUnlockMemo { block_number: 1.into(), @@ -245,7 +250,11 @@ mod tests { }; let expected_action = BridgeUnlockAction { - to: Address::builder().array([1u8; 20]).prefix("astria").build(), + to: Address::builder() + .array([1u8; 20]) + .prefix(ASTRIA_ADDRESS_PREFIX) + .try_build() + .unwrap(), amount: 99, memo: serde_json::to_vec(&BridgeUnlockMemo { block_number: 1.into(), @@ -285,7 +294,11 @@ mod tests { let expected_action = Ics20Withdrawal { denom: denom.clone(), destination_chain_address, - return_address: Address::builder().array([0u8; 20]).prefix("astria").build(), + return_address: Address::builder() + .array([0u8; 20]) + .prefix(ASTRIA_ADDRESS_PREFIX) + .try_build() + .unwrap(), amount: 99, memo: serde_json::to_string(&Ics20WithdrawalMemo { memo: "hello".to_string(), diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs index a5b3c5c029..a75e6a5137 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs @@ -3,13 +3,18 @@ use std::{ path::Path, }; -use astria_core::crypto::SigningKey; +use astria_core::{ + crypto::SigningKey, + primitive::v1::{ + Address, + ASTRIA_ADDRESS_PREFIX, + }, +}; use astria_eyre::eyre::{ self, eyre, WrapErr as _, }; -use sequencer_client::Address; pub(super) struct SequencerKey { pub(super) address: Address, @@ -30,7 +35,7 @@ impl SequencerKey { Ok(Self { address: Address::builder() .array(signing_key.verification_key().address_bytes()) - .prefix("astria") + .prefix(ASTRIA_ADDRESS_PREFIX) .try_build() .wrap_err("failed to construct Sequencer address")?, signing_key, diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/tests.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/tests.rs index cdfe07a34f..21cdcb91fd 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/tests.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/tests.rs @@ -174,7 +174,8 @@ fn make_ics20_withdrawal_action() -> Action { return_address: Address::builder() .array([0u8; 20]) .prefix(ASTRIA_ADDRESS_PREFIX) - .build(), + .try_build() + .unwrap(), amount: 99, memo: serde_json::to_string(&Ics20WithdrawalMemo { memo: "hello".to_string(), @@ -197,7 +198,8 @@ fn make_bridge_unlock_action() -> Action { to: Address::builder() .array([0u8; 20]) .prefix(ASTRIA_ADDRESS_PREFIX) - .build(), + .try_build() + .unwrap(), amount: 99, memo: serde_json::to_vec(&BridgeUnlockMemo { block_number: 1.into(), diff --git a/crates/astria-cli/src/lib.rs b/crates/astria-cli/src/lib.rs index 6c787a1264..2559502a00 100644 --- a/crates/astria-cli/src/lib.rs +++ b/crates/astria-cli/src/lib.rs @@ -14,7 +14,8 @@ pub(crate) fn astria_address(array: [u8; astria_core::primitive::v1::ADDRESS_LEN Address::builder() .array(array) .prefix(ADDRESS_PREFIX) - .build() + .try_build() + .unwrap() } /// Tries to construct an [`Address`] prefixed by `"astria"` from a byte slice. diff --git a/crates/astria-core/src/crypto.rs b/crates/astria-core/src/crypto.rs index be6442e81d..1c3e919635 100644 --- a/crates/astria-core/src/crypto.rs +++ b/crates/astria-core/src/crypto.rs @@ -115,11 +115,13 @@ pub struct VerificationKey { impl VerificationKey { /// Returns the byte encoding of the verification key. + #[must_use] pub fn to_bytes(&self) -> [u8; 32] { self.key.to_bytes() } /// Returns the byte encoding of the verification key. + #[must_use] pub fn as_bytes(&self) -> &[u8; 32] { self.key.as_bytes() } @@ -138,6 +140,7 @@ impl VerificationKey { // Silence the clippy lint because the function body asserts that the panic // cannot happen. #[allow(clippy::missing_panics_doc)] + #[must_use] pub fn address_bytes(&self) -> [u8; ADDRESS_LEN] { fn first_20(array: [u8; 32]) -> [u8; ADDRESS_LEN] { [ diff --git a/crates/astria-core/src/primitive/v1/mod.rs b/crates/astria-core/src/primitive/v1/mod.rs index 3a560a7963..198f7e267f 100644 --- a/crates/astria-core/src/primitive/v1/mod.rs +++ b/crates/astria-core/src/primitive/v1/mod.rs @@ -342,10 +342,13 @@ impl AddressBuilder { } impl<'a, 'b> AddressBuilder, WithPrefix<'b>> { - pub fn build(self) -> Address { - self.try_build().unwrap() - } - + /// Attempts to build an address from the configured prefix and bytes. + /// + /// # Errors + /// Returns an error if one of the following conditions are violated: + /// + 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 { let Self { bytes: WithBytes(bytes), @@ -436,7 +439,7 @@ impl Address { if bech32m.is_empty() { return Self::builder() .slice(inner.as_ref()) - .prefix("astria") + .prefix(ASTRIA_ADDRESS_PREFIX) .try_build(); } if inner.is_empty() { @@ -515,7 +518,7 @@ mod tests { fn assert_wrong_address_bytes(bad_account: &[u8]) { let error = Address::builder() .slice(bad_account) - .prefix("astria") + .prefix(ASTRIA_ADDRESS_PREFIX) .try_build() .expect_err( "converting from an incorrectly sized byte slice succeeded where it should have \ @@ -540,7 +543,11 @@ mod tests { #[test] fn snapshots() { - let address = Address::builder().array([42; 20]).prefix("astria").build(); + let address = Address::builder() + .array([42; 20]) + .prefix(ASTRIA_ADDRESS_PREFIX) + .try_build() + .unwrap(); assert_json_snapshot!(address); } @@ -551,7 +558,8 @@ mod tests { let address = Address::builder() .array([42u8; ADDRESS_LEN]) .prefix(std::str::from_utf8(&long_prefix).unwrap()) - .build(); + .try_build() + .unwrap(); let _ = address.into_raw(); } @@ -611,7 +619,11 @@ mod tests { // allow: `Address::inner` field is deprecated, but we must still check it #![allow(deprecated)] let bytes = [42u8; ADDRESS_LEN]; - let address = Address::builder().array(bytes).prefix("astria").build(); + let address = Address::builder() + .array(bytes) + .prefix(ASTRIA_ADDRESS_PREFIX) + .try_build() + .unwrap(); let output = address.into_raw(); assert!( output.inner.is_empty(), diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs index 4a212e73bf..abe6693e08 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs @@ -397,6 +397,7 @@ mod test { primitive::v1::{ asset::default_native_asset_id, Address, + ASTRIA_ADDRESS_PREFIX, }, protocol::transaction::v1alpha1::action::TransferAction, }; @@ -416,7 +417,11 @@ mod test { ]); let transfer = TransferAction { - to: Address::builder().array([0; 20]).prefix("astria").build(), + to: Address::builder() + .array([0; 20]) + .prefix(ASTRIA_ADDRESS_PREFIX) + .try_build() + .unwrap(), amount: 0, asset_id: default_native_asset_id(), fee_asset_id: default_native_asset_id(), @@ -449,7 +454,11 @@ mod test { ]); let transfer = TransferAction { - to: Address::builder().array([0; 20]).prefix("astria").build(), + to: Address::builder() + .array([0; 20]) + .prefix(ASTRIA_ADDRESS_PREFIX) + .try_build() + .unwrap(), amount: 0, asset_id: default_native_asset_id(), fee_asset_id: default_native_asset_id(), diff --git a/crates/astria-sequencer-client/src/extension_trait.rs b/crates/astria-sequencer-client/src/extension_trait.rs index f67d562174..8a25f5d5c3 100644 --- a/crates/astria-sequencer-client/src/extension_trait.rs +++ b/crates/astria-sequencer-client/src/extension_trait.rs @@ -17,7 +17,7 @@ //! .array(hex_literal::hex!( //! "DEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF" //! )) -//! .prefix("astria") +//! .prefix(ASTRIA_ADDRESS_PREFIX) //! .build(); //! let height = 5u32; //! let balance = client.get_balance(address, height).await?; diff --git a/crates/astria-sequencer-client/src/tests/http.rs b/crates/astria-sequencer-client/src/tests/http.rs index 2234974112..25aa4c1edc 100644 --- a/crates/astria-sequencer-client/src/tests/http.rs +++ b/crates/astria-sequencer-client/src/tests/http.rs @@ -7,6 +7,7 @@ use astria_core::{ default_native_asset_id, }, Address, + ASTRIA_ADDRESS_PREFIX, }, protocol::transaction::v1alpha1::{ action::TransferAction, @@ -49,14 +50,16 @@ const BOB_ADDRESS_BYTES: [u8; 20] = hex!("34fec43c7fcab9aef3b3cf8aba855e41ee69ca fn alice_address() -> Address { Address::builder() .array(ALICE_ADDRESS_BYTES) - .prefix("astria") - .build() + .prefix(ASTRIA_ADDRESS_PREFIX) + .try_build() + .unwrap() } fn bob_address() -> Address { Address::builder() .array(BOB_ADDRESS_BYTES) - .prefix("astria") - .build() + .prefix(ASTRIA_ADDRESS_PREFIX) + .try_build() + .unwrap() } struct MockSequencer { diff --git a/crates/astria-sequencer/src/lib.rs b/crates/astria-sequencer/src/lib.rs index 443576febc..ebbe0bf43c 100644 --- a/crates/astria-sequencer/src/lib.rs +++ b/crates/astria-sequencer/src/lib.rs @@ -36,7 +36,8 @@ pub(crate) fn astria_address(array: [u8; astria_core::primitive::v1::ADDRESS_LEN Address::builder() .array(array) .prefix(ADDRESS_PREFIX) - .build() + .try_build() + .unwrap() } /// Tries to construct an [`Address`] prefixed by `"astria"` from a byte slice. From 5de78bc4e668e502fd9b593a3764fa86e2bff6be Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Thu, 6 Jun 2024 20:19:53 +0200 Subject: [PATCH 11/15] clippy --- crates/astria-core/src/primitive/v1/mod.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/astria-core/src/primitive/v1/mod.rs b/crates/astria-core/src/primitive/v1/mod.rs index 198f7e267f..bc23bf50f5 100644 --- a/crates/astria-core/src/primitive/v1/mod.rs +++ b/crates/astria-core/src/primitive/v1/mod.rs @@ -313,6 +313,7 @@ impl AddressBuilder { } impl AddressBuilder { + #[must_use = "the builder must be used to construct an address to be useful"] pub fn array(self, array: [u8; ADDRESS_LEN]) -> AddressBuilder, TPrefix> { AddressBuilder { bytes: WithBytes(BytesInner::Array(array)), @@ -320,6 +321,7 @@ impl AddressBuilder { } } + #[must_use = "the builder must be used to construct an address to be useful"] pub fn slice<'a, T: Into>>( self, bytes: T, @@ -330,6 +332,7 @@ impl AddressBuilder { } } + #[must_use = "the builder must be used to construct an address to be useful"] pub fn prefix<'a, T: Into>>( self, prefix: T, @@ -371,19 +374,16 @@ impl<'a, 'b> AddressBuilder, WithPrefix<'b>> { #[cfg_attr(feature = "serde", derive(serde::Serialize))] #[cfg_attr(feature = "serde", serde(into = "raw::Address"))] pub struct Address { - prefix: bech32::Hrp, bytes: [u8; ADDRESS_LEN], + prefix: bech32::Hrp, } impl Address { + #[must_use = "the builder must be used to construct an address to be useful"] pub fn builder() -> AddressBuilder { AddressBuilder::new() } - // #[must_use] - // pub fn get(self) -> [u8; ADDRESS_LEN] { - // self.0 - // } #[must_use] pub fn bytes(self) -> [u8; ADDRESS_LEN] { self.bytes From 6b70b86db87a832ffe46909f5d63dfad5015e8fe Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Thu, 6 Jun 2024 20:24:41 +0200 Subject: [PATCH 12/15] fix doctest, clippy --- .../src/withdrawer/ethereum/convert.rs | 4 ++-- crates/astria-cli/src/commands/sequencer.rs | 2 +- crates/astria-sequencer-client/src/extension_trait.rs | 8 ++++++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/convert.rs b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/convert.rs index d1cf0c795c..ba0c6abf8c 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/convert.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/ethereum/convert.rs @@ -99,7 +99,7 @@ fn event_to_bridge_unlock( }; let action = BridgeUnlockAction { to: Address::builder() - .array(event.destination_chain_address.to_fixed_bytes().into()) + .array(event.destination_chain_address.to_fixed_bytes()) .prefix(ASTRIA_ADDRESS_PREFIX) .try_build() .wrap_err("failed to construct destination address")?, @@ -134,7 +134,7 @@ fn event_to_ics20_withdrawal( // TODO: make this configurable const ICS20_WITHDRAWAL_TIMEOUT: Duration = Duration::from_secs(300); - let sender = event.sender.to_fixed_bytes().into(); + let sender = event.sender.to_fixed_bytes(); let denom = rollup_asset_denom.clone(); let (_, channel) = denom diff --git a/crates/astria-cli/src/commands/sequencer.rs b/crates/astria-cli/src/commands/sequencer.rs index 22e592a503..1858233e6f 100644 --- a/crates/astria-cli/src/commands/sequencer.rs +++ b/crates/astria-cli/src/commands/sequencer.rs @@ -63,7 +63,7 @@ fn get_private_key_pretty(signing_key: &SigningKey) -> String { /// Get the address from the signing key fn get_address_pretty(signing_key: &SigningKey) -> String { - hex::encode(&signing_key.verification_key().address_bytes()) + hex::encode(signing_key.verification_key().address_bytes()) } /// Generates a new ED25519 keypair and prints the public key, private key, and address diff --git a/crates/astria-sequencer-client/src/extension_trait.rs b/crates/astria-sequencer-client/src/extension_trait.rs index 8a25f5d5c3..5c8966c72e 100644 --- a/crates/astria-sequencer-client/src/extension_trait.rs +++ b/crates/astria-sequencer-client/src/extension_trait.rs @@ -8,7 +8,10 @@ //! The example below works with the feature `"http"` set. //! ```no_run //! # tokio_test::block_on(async { -//! use astria_core::primitive::v1::Address; +//! use astria_core::primitive::v1::{ +//! Address, +//! ASTRIA_ADDRESS_PREFIX, +//! }; //! use astria_sequencer_client::SequencerClientExt as _; //! use tendermint_rpc::HttpClient; //! @@ -18,7 +21,8 @@ //! "DEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF" //! )) //! .prefix(ASTRIA_ADDRESS_PREFIX) -//! .build(); +//! .try_build() +//! .unwrap(); //! let height = 5u32; //! let balance = client.get_balance(address, height).await?; //! println!("{balance:?}"); From 7ccb839eeb19d83a7b7e70861e776842e3becb2e Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Fri, 7 Jun 2024 20:25:10 +0200 Subject: [PATCH 13/15] remove commented out code --- crates/astria-core/src/primitive/v1/mod.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/crates/astria-core/src/primitive/v1/mod.rs b/crates/astria-core/src/primitive/v1/mod.rs index bc23bf50f5..fc28e754f8 100644 --- a/crates/astria-core/src/primitive/v1/mod.rs +++ b/crates/astria-core/src/primitive/v1/mod.rs @@ -455,12 +455,6 @@ impl AsRef<[u8]> for Address { } } -// impl From<[u8; ADDRESS_LEN]> for Address { -// fn from(inner: [u8; ADDRESS_LEN]) -> Self { -// Self(inner) -// } -// } - impl From
for raw::Address { fn from(value: Address) -> Self { value.into_raw() From e81d36009be00265adfb6b08bab59c3f57a19b5c Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Fri, 7 Jun 2024 22:08:27 +0200 Subject: [PATCH 14/15] provide address method on signed tx, stricter builder for transaction params --- .../src/withdrawer/submitter/mod.rs | 16 +- crates/astria-cli/src/commands/sequencer.rs | 9 +- crates/astria-composer/src/executor/mod.rs | 22 +- .../tests/blackbox/helper/mod.rs | 5 +- crates/astria-core/src/primitive/v1/mod.rs | 36 ++- crates/astria-core/src/protocol/test_utils.rs | 9 +- .../src/protocol/transaction/v1alpha1/mod.rs | 153 ++++++++++-- .../astria-sequencer-client/src/tests/http.rs | 9 +- crates/astria-sequencer/src/app/mod.rs | 2 +- crates/astria-sequencer/src/app/test_utils.rs | 9 +- crates/astria-sequencer/src/app/tests_app.rs | 63 ++--- .../src/app/tests_breaking_changes.rs | 27 +- .../src/app/tests_execute_transaction.rs | 234 ++++++++++-------- crates/astria-sequencer/src/mempool.rs | 27 +- .../src/proposal/commitment.rs | 27 +- .../astria-sequencer/src/service/consensus.rs | 9 +- .../src/transaction/checks.rs | 28 +-- .../astria-sequencer/src/transaction/mod.rs | 13 +- 18 files changed, 442 insertions(+), 256 deletions(-) diff --git a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs index ad90e9e941..dd475a43c8 100644 --- a/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs +++ b/crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs @@ -123,10 +123,14 @@ impl Submitter { let unsigned = UnsignedTransaction { actions, - params: TransactionParams { - nonce, - chain_id: self.sequencer_chain_id.clone(), - }, + params: TransactionParams::builder() + .nonce(nonce) + .chain_id(&self.sequencer_chain_id) + .try_build() + .context( + "failed to construct transcation parameters from latest nonce and configured \ + sequencer chain ID", + )?, }; // sign transaction @@ -234,7 +238,7 @@ async fn get_latest_nonce( name = "submit_tx", skip_all, fields( - nonce = tx.unsigned_transaction().params.nonce, + nonce = tx.nonce(), transaction.hash = %telemetry::display::hex(&tx.sha256_of_proto_encoding()), ) )] @@ -243,7 +247,7 @@ async fn submit_tx( tx: SignedTransaction, state: Arc, ) -> eyre::Result { - let nonce = tx.unsigned_transaction().params.nonce; + let nonce = tx.nonce(); metrics::gauge!(crate::metrics_init::CURRENT_NONCE).set(nonce); let start = std::time::Instant::now(); debug!("submitting signed transaction to sequencer"); diff --git a/crates/astria-cli/src/commands/sequencer.rs b/crates/astria-cli/src/commands/sequencer.rs index 1858233e6f..44f944b898 100644 --- a/crates/astria-cli/src/commands/sequencer.rs +++ b/crates/astria-cli/src/commands/sequencer.rs @@ -452,10 +452,11 @@ async fn submit_transaction( .wrap_err("failed to get nonce")?; let tx = UnsignedTransaction { - params: TransactionParams { - nonce: nonce_res.nonce, - chain_id, - }, + params: TransactionParams::builder() + .nonce(nonce_res.nonce) + .chain_id(chain_id) + .try_build() + .wrap_err("failed to construct transaction params from provided chain ID")?, actions: vec![action], } .into_signed(&sequencer_key); diff --git a/crates/astria-composer/src/executor/mod.rs b/crates/astria-composer/src/executor/mod.rs index 4c2dc7c23c..131446583b 100644 --- a/crates/astria-composer/src/executor/mod.rs +++ b/crates/astria-composer/src/executor/mod.rs @@ -467,7 +467,7 @@ async fn get_latest_nonce( name = "submit signed transaction", skip_all, fields( - nonce = tx.unsigned_transaction().params.nonce, + nonce = tx.nonce(), transaction.hash = hex::encode(sha256(&tx.to_raw().encode_to_vec())), ) )] @@ -475,7 +475,7 @@ async fn submit_tx( client: sequencer_client::HttpClient, tx: SignedTransaction, ) -> eyre::Result { - let nonce = tx.unsigned_transaction().params.nonce; + let nonce = tx.nonce(); metrics::gauge!(crate::metrics_init::CURRENT_NONCE).set(nonce); // TODO: change to info and log tx hash (to match info log in `SubmitFut`'s response handling @@ -568,10 +568,11 @@ impl Future for SubmitFut { let new_state = match this.state.project() { SubmitStateProj::NotStarted => { - let params = TransactionParams { - nonce: *this.nonce, - chain_id: this.chain_id.clone(), - }; + let params = TransactionParams::builder() + .nonce(*this.nonce) + .chain_id(&*this.chain_id) + .try_build() + .expect("configured chain ID is valid"); let tx = UnsignedTransaction { actions: this.bundle.clone().into_actions(), params, @@ -653,10 +654,11 @@ impl Future for SubmitFut { } => match ready!(fut.poll(cx)) { Ok(nonce) => { *this.nonce = nonce; - let params = TransactionParams { - nonce: *this.nonce, - chain_id: this.chain_id.clone(), - }; + let params = TransactionParams::builder() + .nonce(*this.nonce) + .chain_id(&*this.chain_id) + .try_build() + .expect("configured chain ID is valid"); let tx = UnsignedTransaction { actions: this.bundle.clone().into_actions(), params, diff --git a/crates/astria-composer/tests/blackbox/helper/mod.rs b/crates/astria-composer/tests/blackbox/helper/mod.rs index fe09219ef9..ea802b2896 100644 --- a/crates/astria-composer/tests/blackbox/helper/mod.rs +++ b/crates/astria-composer/tests/blackbox/helper/mod.rs @@ -177,10 +177,7 @@ fn rollup_id_nonce_from_request(request: &Request) -> (RollupId, u32) { panic!("mocked sequencer expected a sequence action"); }; - ( - sequence_action.rollup_id, - signed_tx.unsigned_transaction().params.nonce, - ) + (sequence_action.rollup_id, signed_tx.nonce()) } /// Deserializes the bytes contained in a `tx_sync::Request` to a signed sequencer transaction and diff --git a/crates/astria-core/src/primitive/v1/mod.rs b/crates/astria-core/src/primitive/v1/mod.rs index fc28e754f8..26c958aca3 100644 --- a/crates/astria-core/src/primitive/v1/mod.rs +++ b/crates/astria-core/src/primitive/v1/mod.rs @@ -313,7 +313,7 @@ impl AddressBuilder { } impl AddressBuilder { - #[must_use = "the builder must be used to construct an address to be useful"] + #[must_use = "the builder must be built to construct an address to be useful"] pub fn array(self, array: [u8; ADDRESS_LEN]) -> AddressBuilder, TPrefix> { AddressBuilder { bytes: WithBytes(BytesInner::Array(array)), @@ -321,7 +321,7 @@ impl AddressBuilder { } } - #[must_use = "the builder must be used to construct an address to be useful"] + #[must_use = "the builder must be built to construct an address to be useful"] pub fn slice<'a, T: Into>>( self, bytes: T, @@ -332,7 +332,7 @@ impl AddressBuilder { } } - #[must_use = "the builder must be used to construct an address to be useful"] + #[must_use = "the builder must be built to construct an address to be useful"] pub fn prefix<'a, T: Into>>( self, prefix: T, @@ -370,6 +370,36 @@ impl<'a, 'b> AddressBuilder, WithPrefix<'b>> { } } +// Private setters only used within this crate to not leak bech32 +impl AddressBuilder { + pub(crate) fn array__( + self, + array: [u8; ADDRESS_LEN], + ) -> AddressBuilder<[u8; ADDRESS_LEN], TPrefix> { + AddressBuilder { + bytes: array, + prefix: self.prefix, + } + } + + pub(crate) fn hrp__(self, prefix: bech32::Hrp) -> AddressBuilder { + AddressBuilder { + bytes: self.bytes, + prefix, + } + } +} + +// private builder to not leak bech32 +impl AddressBuilder<[u8; ADDRESS_LEN], bech32::Hrp> { + pub(crate) fn build(self) -> Address { + Address { + bytes: self.bytes, + prefix: self.prefix, + } + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] #[cfg_attr(feature = "serde", derive(serde::Serialize))] #[cfg_attr(feature = "serde", serde(into = "raw::Address"))] diff --git a/crates/astria-core/src/protocol/test_utils.rs b/crates/astria-core/src/protocol/test_utils.rs index 4fa099e202..8122232846 100644 --- a/crates/astria-core/src/protocol/test_utils.rs +++ b/crates/astria-core/src/protocol/test_utils.rs @@ -106,10 +106,11 @@ impl ConfigureSequencerBlock { } else { let unsigned_transaction = UnsignedTransaction { actions, - params: TransactionParams { - nonce: 1, - chain_id: chain_id.clone(), - }, + params: TransactionParams::builder() + .nonce(1) + .chain_id(chain_id.clone()) + .try_build() + .unwrap(), }; vec![unsigned_transaction.into_signed(&signing_key)] }; diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs index abe6693e08..4c26a68368 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs @@ -4,11 +4,14 @@ use prost::{ }; use super::raw; -use crate::crypto::{ - self, - Signature, - SigningKey, - VerificationKey, +use crate::{ + crypto::{ + self, + Signature, + SigningKey, + VerificationKey, + }, + primitive::v1::Address, }; pub mod action; @@ -76,6 +79,13 @@ pub struct SignedTransaction { } impl SignedTransaction { + pub fn address(&self) -> Address { + crate::primitive::v1::Address::builder() + .array__(self.verification_key.address_bytes()) + .hrp__(self.transaction.hrp()) + .build() + } + /// Returns the transaction hash. /// /// The transaction hash is calculated by protobuf-encoding the transaction @@ -203,9 +213,13 @@ impl SignedTransaction { &self.transaction } + pub fn chain_id(&self) -> &str { + self.transaction.chain_id() + } + #[must_use] pub fn nonce(&self) -> u32 { - self.transaction.params.nonce + self.transaction.nonce() } } @@ -217,6 +231,18 @@ pub struct UnsignedTransaction { } impl UnsignedTransaction { + fn hrp(&self) -> bech32::Hrp { + self.params.hrp + } + + pub fn nonce(&self) -> u32 { + self.params.nonce + } + + pub fn chain_id(&self) -> &str { + &self.params.chain_id + } + #[must_use] pub fn into_signed(self, signing_key: &SigningKey) -> SignedTransaction { let bytes = self.to_raw().encode_to_vec(); @@ -283,7 +309,8 @@ impl UnsignedTransaction { let Some(params) = params else { return Err(UnsignedTransactionError::unset_params()); }; - let params = TransactionParams::from_raw(params); + let params = TransactionParams::try_from_raw(params) + .map_err(UnsignedTransactionError::transaction_params)?; let actions: Vec<_> = actions .into_iter() .map(Action::try_from_raw) @@ -335,6 +362,12 @@ impl UnsignedTransactionError { fn decode_any(inner: prost::DecodeError) -> Self { Self(UnsignedTransactionErrorKind::DecodeAny(inner)) } + + fn transaction_params(source: TransactionParamsError) -> Self { + Self(UnsignedTransactionErrorKind::TransactionParams { + source, + }) + } } #[derive(Debug, thiserror::Error)] @@ -354,21 +387,103 @@ enum UnsignedTransactionErrorKind { raw::UnsignedTransaction::type_url() )] DecodeAny(#[source] prost::DecodeError), + #[error("`params` field was invalid")] + TransactionParams { source: TransactionParamsError }, +} + +#[derive(Debug, thiserror::Error)] +#[error(transparent)] +pub struct TransactionParamsError(TransactionParamsErrorKind); + +impl TransactionParamsError { + fn chain_id_not_bech32_compatible(source: bech32::primitives::hrp::Error) -> Self { + Self(TransactionParamsErrorKind::ChainIdNotBech32Compatible { + source, + }) + } +} + +#[derive(Debug, thiserror::Error)] +enum TransactionParamsErrorKind { + #[error("the name extracted from the chain ID cannot be used as an address prefix")] + ChainIdNotBech32Compatible { + source: bech32::primitives::hrp::Error, + }, +} + +pub struct TransactionParamsBuilder> { + nonce: u32, + chain_id: TChainId, +} + +impl TransactionParamsBuilder { + fn new() -> Self { + Self { + nonce: 0, + chain_id: "".into(), + } + } +} + +impl TransactionParamsBuilder { + pub fn chain_id<'a, T: Into>>( + self, + chain_id: T, + ) -> TransactionParamsBuilder> { + TransactionParamsBuilder { + chain_id: chain_id.into(), + nonce: self.nonce, + } + } + + pub fn nonce(self, nonce: u32) -> Self { + Self { + nonce, + ..self + } + } +} + +impl<'a> TransactionParamsBuilder> { + pub fn try_build(self) -> Result { + let Self { + nonce, + chain_id, + } = self; + let chain_id = chain_id.as_ref().trim().to_string(); + let hrp = bech32::Hrp::parse( + chain_id + .split_once('-') + .map(|tup| tup.0) + .unwrap_or(&chain_id), + ) + .map_err(TransactionParamsError::chain_id_not_bech32_compatible)?; + Ok(TransactionParams { + nonce, + chain_id, + hrp, + }) + } } #[derive(Clone, Debug)] -#[allow(clippy::module_name_repetitions)] pub struct TransactionParams { - pub nonce: u32, - pub chain_id: String, + nonce: u32, + chain_id: String, + hrp: bech32::Hrp, } impl TransactionParams { + pub fn builder() -> TransactionParamsBuilder { + TransactionParamsBuilder::new() + } + #[must_use] pub fn into_raw(self) -> raw::TransactionParams { let Self { nonce, chain_id, + .. } = self; raw::TransactionParams { nonce, @@ -377,16 +492,12 @@ impl TransactionParams { } /// Convert from a raw protobuf [`raw::UnsignedTransaction`]. - #[must_use] - pub fn from_raw(proto: raw::TransactionParams) -> Self { + pub fn try_from_raw(proto: raw::TransactionParams) -> Result { let raw::TransactionParams { nonce, chain_id, } = proto; - Self { - nonce, - chain_id, - } + Self::builder().nonce(nonce).chain_id(chain_id).try_build() } } @@ -427,10 +538,11 @@ mod test { fee_asset_id: default_native_asset_id(), }; - let params = TransactionParams { + let params = TransactionParams::try_from_raw(raw::TransactionParams { nonce: 1, chain_id: "test-1".to_string(), - }; + }) + .unwrap(); let unsigned = UnsignedTransaction { actions: vec![transfer.into()], params, @@ -464,10 +576,11 @@ mod test { fee_asset_id: default_native_asset_id(), }; - let params = TransactionParams { + let params = TransactionParams::try_from_raw(raw::TransactionParams { nonce: 1, chain_id: "test-1".to_string(), - }; + }) + .unwrap(); let unsigned = UnsignedTransaction { actions: vec![transfer.into()], params, diff --git a/crates/astria-sequencer-client/src/tests/http.rs b/crates/astria-sequencer-client/src/tests/http.rs index 25aa4c1edc..f4bbca6ebd 100644 --- a/crates/astria-sequencer-client/src/tests/http.rs +++ b/crates/astria-sequencer-client/src/tests/http.rs @@ -158,10 +158,11 @@ fn create_signed_transaction() -> SignedTransaction { .into(), ]; UnsignedTransaction { - params: TransactionParams { - nonce: 1, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(1) + .chain_id("test") + .try_build() + .unwrap(), actions, } .into_signed(&alice_key) diff --git a/crates/astria-sequencer/src/app/mod.rs b/crates/astria-sequencer/src/app/mod.rs index b00c02b9ec..996a786ea4 100644 --- a/crates/astria-sequencer/src/app/mod.rs +++ b/crates/astria-sequencer/src/app/mod.rs @@ -961,7 +961,7 @@ impl App { /// Executes a signed transaction. #[instrument(name = "App::execute_transaction", skip_all, fields( signed_transaction_hash = %telemetry::display::base64(&signed_tx.sha256_of_proto_encoding()), - sender = %telemetry::display::base64(&signed_tx.verification_key().address_bytes()), + sender = %signed_tx.address(), ))] pub(crate) async fn execute_transaction( &mut self, diff --git a/crates/astria-sequencer/src/app/test_utils.rs b/crates/astria-sequencer/src/app/test_utils.rs index 6ea666eb29..72e27e0a85 100644 --- a/crates/astria-sequencer/src/app/test_utils.rs +++ b/crates/astria-sequencer/src/app/test_utils.rs @@ -136,10 +136,11 @@ pub(crate) async fn initialize_app( pub(crate) fn get_mock_tx(nonce: u32) -> SignedTransaction { let (alice_signing_key, _) = get_alice_signing_key_and_address(); let tx = UnsignedTransaction { - params: TransactionParams { - nonce, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(nonce) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![ SequenceAction { rollup_id: RollupId::from_unhashed_bytes([0; 32]), diff --git a/crates/astria-sequencer/src/app/tests_app.rs b/crates/astria-sequencer/src/app/tests_app.rs index da7b266943..6317c3232f 100644 --- a/crates/astria-sequencer/src/app/tests_app.rs +++ b/crates/astria-sequencer/src/app/tests_app.rs @@ -236,10 +236,11 @@ async fn app_transfer_block_fees_to_sudo() { let bob_address = address_from_hex_string(BOB_ADDRESS); let amount = 333_333; let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![ TransferAction { to: bob_address, @@ -326,10 +327,11 @@ async fn app_create_sequencer_block_with_sequenced_data_and_deposits() { fee_asset_id: asset_id, }; let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![lock_action.into(), sequence_action.into()], }; @@ -416,10 +418,11 @@ async fn app_execution_results_match_proposal_vs_after_proposal() { fee_asset_id: asset_id, }; let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![lock_action.into(), sequence_action.into()], }; @@ -540,10 +543,11 @@ async fn app_prepare_proposal_cometbft_max_bytes_overflow_ok() { // create txs which will cause cometBFT overflow let (alice_signing_key, _) = get_alice_signing_key_and_address(); let tx_pass = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![ SequenceAction { rollup_id: RollupId::from([1u8; 32]), @@ -555,10 +559,11 @@ async fn app_prepare_proposal_cometbft_max_bytes_overflow_ok() { } .into_signed(&alice_signing_key); let tx_overflow = UnsignedTransaction { - params: TransactionParams { - nonce: 1, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(1) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![ SequenceAction { rollup_id: RollupId::from([1u8; 32]), @@ -613,10 +618,11 @@ async fn app_prepare_proposal_sequencer_max_bytes_overflow_ok() { // create txs which will cause sequencer overflow (max is currently 256_000 bytes) let (alice_signing_key, _) = get_alice_signing_key_and_address(); let tx_pass = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![ SequenceAction { rollup_id: RollupId::from([1u8; 32]), @@ -628,10 +634,11 @@ async fn app_prepare_proposal_sequencer_max_bytes_overflow_ok() { } .into_signed(&alice_signing_key); let tx_overflow = UnsignedTransaction { - params: TransactionParams { - nonce: 1, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(1) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![ SequenceAction { rollup_id: RollupId::from([1u8; 32]), diff --git a/crates/astria-sequencer/src/app/tests_breaking_changes.rs b/crates/astria-sequencer/src/app/tests_breaking_changes.rs index 54630124f4..b48110445a 100644 --- a/crates/astria-sequencer/src/app/tests_breaking_changes.rs +++ b/crates/astria-sequencer/src/app/tests_breaking_changes.rs @@ -103,10 +103,11 @@ async fn app_finalize_block_snapshot() { fee_asset_id: asset_id, }; let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![lock_action.into(), sequence_action.into()], }; @@ -196,10 +197,11 @@ async fn app_execute_transaction_with_every_action_snapshot() { app.apply(state_tx); let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![ TransferAction { to: bob_address, @@ -248,10 +250,11 @@ async fn app_execute_transaction_with_every_action_snapshot() { // execute BridgeUnlock action let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![ BridgeUnlockAction { to: bob_address, diff --git a/crates/astria-sequencer/src/app/tests_execute_transaction.rs b/crates/astria-sequencer/src/app/tests_execute_transaction.rs index f2be1ffd24..716ad2a663 100644 --- a/crates/astria-sequencer/src/app/tests_execute_transaction.rs +++ b/crates/astria-sequencer/src/app/tests_execute_transaction.rs @@ -53,10 +53,11 @@ async fn app_execute_transaction_transfer() { let bob_address = address_from_hex_string(BOB_ADDRESS); let value = 333_333; let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![ TransferAction { to: bob_address, @@ -110,10 +111,11 @@ async fn app_execute_transaction_transfer_not_native_token() { // transfer funds from Alice to Bob; use native token for fee payment let bob_address = address_from_hex_string(BOB_ADDRESS); let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![ TransferAction { to: bob_address, @@ -176,10 +178,11 @@ async fn app_execute_transaction_transfer_balance_too_low_for_fee() { // 0-value transfer; only fee is deducted from sender let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![ TransferAction { to: bob, @@ -216,10 +219,11 @@ async fn app_execute_transaction_sequence() { let fee = calculate_fee_from_state(&data, &app.state).await.unwrap(); let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![ SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), @@ -253,10 +257,11 @@ async fn app_execute_transaction_invalid_fee_asset() { let fee_asset_id = asset::Id::from_denom("test"); let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![ SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), @@ -294,10 +299,11 @@ async fn app_execute_transaction_validator_update() { }; let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![Action::ValidatorUpdate(update.clone())], }; @@ -327,10 +333,11 @@ async fn app_execute_transaction_ibc_relayer_change_addition() { let mut app = initialize_app(Some(genesis_state), vec![]).await; let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![IbcRelayerChangeAction::Addition(alice_address).into()], }; @@ -357,10 +364,11 @@ async fn app_execute_transaction_ibc_relayer_change_deletion() { let mut app = initialize_app(Some(genesis_state), vec![]).await; let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![IbcRelayerChangeAction::Removal(alice_address).into()], }; @@ -387,10 +395,11 @@ async fn app_execute_transaction_ibc_relayer_change_invalid() { let mut app = initialize_app(Some(genesis_state), vec![]).await; let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![IbcRelayerChangeAction::Removal(alice_address).into()], }; @@ -417,10 +426,11 @@ async fn app_execute_transaction_sudo_address_change() { let new_address = address_from_hex_string(BOB_ADDRESS); let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![Action::SudoAddressChange(SudoAddressChangeAction { new_address, })], @@ -452,10 +462,11 @@ async fn app_execute_transaction_sudo_address_change_error() { let mut app = initialize_app(Some(genesis_state), vec![]).await; let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![Action::SudoAddressChange(SudoAddressChangeAction { new_address: alice_address, })], @@ -492,10 +503,11 @@ async fn app_execute_transaction_fee_asset_change_addition() { let new_asset = asset::Id::from_denom("test"); let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![Action::FeeAssetChange(FeeAssetChangeAction::Addition( new_asset, ))], @@ -531,10 +543,11 @@ async fn app_execute_transaction_fee_asset_change_removal() { let mut app = initialize_app(Some(genesis_state), vec![]).await; let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![Action::FeeAssetChange(FeeAssetChangeAction::Removal( test_asset.id(), ))], @@ -571,10 +584,11 @@ async fn app_execute_transaction_fee_asset_change_invalid() { let mut app = initialize_app(Some(genesis_state), vec![]).await; let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![Action::FeeAssetChange(FeeAssetChangeAction::Removal( get_native_asset().id(), ))], @@ -609,10 +623,11 @@ async fn app_execute_transaction_init_bridge_account_ok() { fee_asset_id: asset_id, }; let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![action.into()], }; @@ -664,10 +679,11 @@ async fn app_execute_transaction_init_bridge_account_account_already_registered( fee_asset_id: asset_id, }; let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![action.into()], }; @@ -680,10 +696,11 @@ async fn app_execute_transaction_init_bridge_account_account_already_registered( fee_asset_id: asset_id, }; let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 1, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![action.into()], }; @@ -716,10 +733,11 @@ async fn app_execute_transaction_bridge_lock_action_ok() { destination_chain_address: "nootwashere".to_string(), }; let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![action.into()], }; @@ -794,10 +812,11 @@ async fn app_execute_transaction_bridge_lock_action_invalid_for_eoa() { destination_chain_address: "nootwashere".to_string(), }; let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![action.into()], }; @@ -814,10 +833,11 @@ async fn app_execute_transaction_invalid_nonce() { // create tx with invalid nonce 1 let data = b"hello world".to_vec(); let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 1, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(1) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![ SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), @@ -860,10 +880,11 @@ async fn app_execute_transaction_invalid_chain_id() { // create tx with invalid nonce 1 let data = b"hello world".to_vec(); let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "wrong-chain".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("wrong-chain") + .try_build() + .unwrap(), actions: vec![ SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), @@ -919,10 +940,11 @@ async fn app_stateful_check_fails_insufficient_total_balance() { // transfer just enough to cover single sequence fee with data let signed_tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![ TransferAction { to: keypair_address, @@ -940,10 +962,11 @@ async fn app_stateful_check_fails_insufficient_total_balance() { // build double transfer exceeding balance let signed_tx_fail = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![ SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), @@ -971,10 +994,11 @@ async fn app_stateful_check_fails_insufficient_total_balance() { // build single transfer to see passes let signed_tx_pass = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![ SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), @@ -1026,10 +1050,11 @@ async fn app_execute_transaction_bridge_lock_unlock_action_ok() { destination_chain_address: "nootwashere".to_string(), }; let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![action.into()], }; @@ -1047,10 +1072,11 @@ async fn app_execute_transaction_bridge_lock_unlock_action_ok() { }; let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![action.into()], }; diff --git a/crates/astria-sequencer/src/mempool.rs b/crates/astria-sequencer/src/mempool.rs index 578895784b..1da336b3c1 100644 --- a/crates/astria-sequencer/src/mempool.rs +++ b/crates/astria-sequencer/src/mempool.rs @@ -262,10 +262,11 @@ fn dummy_signed_tx() -> (Arc, Address) { static TX: OnceLock<(Arc, Address)> = OnceLock::new(); let (signed_tx, address) = TX.get_or_init(|| { let actions = vec![]; - let params = TransactionParams { - nonce: 0, - chain_id: String::new(), - }; + let params = TransactionParams::builder() + .nonce(0) + .chain_id("dummy") + .try_build() + .expect("all params are valid"); let signing_key = SigningKey::from([0; 32]); let address = crate::astria_address(signing_key.verification_key().address_bytes()); let unsigned_tx = UnsignedTransaction { @@ -462,10 +463,11 @@ mod test { let other_mock_tx = |nonce: u32| -> SignedTransaction { let actions = get_mock_tx(0).actions().to_vec(); UnsignedTransaction { - params: TransactionParams { - nonce, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(nonce) + .chain_id("test") + .try_build() + .unwrap(), actions, } .into_signed(&other_signing_key) @@ -533,10 +535,11 @@ mod test { let other_mock_tx = |nonce: u32| -> SignedTransaction { let actions = get_mock_tx(0).actions().to_vec(); UnsignedTransaction { - params: TransactionParams { - nonce, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(nonce) + .chain_id("test") + .try_build() + .unwrap(), actions, } .into_signed(&other_signing_key) diff --git a/crates/astria-sequencer/src/proposal/commitment.rs b/crates/astria-sequencer/src/proposal/commitment.rs index 5e5b0d820a..46f018a9f4 100644 --- a/crates/astria-sequencer/src/proposal/commitment.rs +++ b/crates/astria-sequencer/src/proposal/commitment.rs @@ -128,10 +128,11 @@ mod test { let signing_key = SigningKey::new(OsRng); let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test-chain-1".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test-chain-1") + .try_build() + .unwrap(), actions: vec![sequence_action.clone().into(), transfer_action.into()], }; @@ -144,10 +145,11 @@ mod test { let signing_key = SigningKey::new(OsRng); let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test-chain-1".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test-chain-1") + .try_build() + .unwrap(), actions: vec![sequence_action.into()], }; @@ -183,10 +185,11 @@ mod test { let signing_key = SigningKey::new(OsRng); let tx = UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test-chain-1".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test-chain-1") + .try_build() + .unwrap(), actions: vec![sequence_action.into(), transfer_action.into()], }; diff --git a/crates/astria-sequencer/src/service/consensus.rs b/crates/astria-sequencer/src/service/consensus.rs index 357bfa5f95..acac6ed0db 100644 --- a/crates/astria-sequencer/src/service/consensus.rs +++ b/crates/astria-sequencer/src/service/consensus.rs @@ -252,10 +252,11 @@ mod test { fn make_unsigned_tx() -> UnsignedTransaction { UnsignedTransaction { - params: TransactionParams { - nonce: 0, - chain_id: "test".to_string(), - }, + params: TransactionParams::builder() + .nonce(0) + .chain_id("test") + .try_build() + .unwrap(), actions: vec![ SequenceAction { rollup_id: RollupId::from_unhashed_bytes(b"testchainid"), diff --git a/crates/astria-sequencer/src/transaction/checks.rs b/crates/astria-sequencer/src/transaction/checks.rs index 89423cd248..f0db1ecbef 100644 --- a/crates/astria-sequencer/src/transaction/checks.rs +++ b/crates/astria-sequencer/src/transaction/checks.rs @@ -36,10 +36,7 @@ pub(crate) async fn check_nonce_mempool( .get_account_nonce(signer_address) .await .context("failed to get account nonce")?; - ensure!( - tx.unsigned_transaction().params.nonce >= curr_nonce, - "nonce already used by account" - ); + ensure!(tx.nonce() >= curr_nonce, "nonce already used by account"); Ok(()) } @@ -51,10 +48,7 @@ pub(crate) async fn check_chain_id_mempool( .get_chain_id() .await .context("failed to get chain id")?; - ensure!( - tx.unsigned_transaction().params.chain_id == chain_id.as_str(), - "chain id mismatch" - ); + ensure!(tx.chain_id() == chain_id.as_str(), "chain id mismatch"); Ok(()) } @@ -344,10 +338,11 @@ mod test { }), ]; - let params = TransactionParams { - nonce: 0, - chain_id: "test-chain-id".to_string(), - }; + let params = TransactionParams::builder() + .nonce(0) + .chain_id("test-chain-id") + .try_build() + .unwrap(); let tx = UnsignedTransaction { actions, params, @@ -406,10 +401,11 @@ mod test { }), ]; - let params = TransactionParams { - nonce: 0, - chain_id: "test-chain-id".to_string(), - }; + let params = TransactionParams::builder() + .nonce(0) + .chain_id("test-chain-id") + .try_build() + .unwrap(); let tx = UnsignedTransaction { actions, params, diff --git a/crates/astria-sequencer/src/transaction/mod.rs b/crates/astria-sequencer/src/transaction/mod.rs index b96e61aede..cd1d0b63f2 100644 --- a/crates/astria-sequencer/src/transaction/mod.rs +++ b/crates/astria-sequencer/src/transaction/mod.rs @@ -184,17 +184,14 @@ impl ActionHandler for UnsignedTransaction { // Transactions must match the chain id of the node. let chain_id = state.get_chain_id().await?; ensure!( - self.params.chain_id == chain_id.as_str(), - InvalidChainId(self.params.chain_id.clone()) + self.chain_id() == chain_id.as_str(), + InvalidChainId(self.chain_id().to_string()) ); // Nonce should be equal to the number of executed transactions before this tx. // First tx has nonce 0. let curr_nonce = state.get_account_nonce(from).await?; - ensure!( - curr_nonce == self.params.nonce, - InvalidNonce(self.params.nonce) - ); + ensure!(curr_nonce == self.nonce(), InvalidNonce(self.nonce())); // Should have enough balance to cover all actions. check_balance_for_total_fees(self, from, state).await?; @@ -263,8 +260,8 @@ impl ActionHandler for UnsignedTransaction { #[instrument( skip_all, fields( - nonce = self.params.nonce, - from = from.to_string(), + nonce = self.nonce(), + from = %from, ) )] async fn execute(&self, state: &mut S, from: Address) -> anyhow::Result<()> { From 4eca1f52a28048fdf77efd9f817d1abcc2dc9b1d Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Fri, 7 Jun 2024 22:13:35 +0200 Subject: [PATCH 15/15] clippy --- .../src/protocol/transaction/v1alpha1/mod.rs | 22 +++++++++++++------ 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs b/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs index 4c26a68368..9e40c91e99 100644 --- a/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs +++ b/crates/astria-core/src/protocol/transaction/v1alpha1/mod.rs @@ -235,10 +235,12 @@ impl UnsignedTransaction { self.params.hrp } + #[must_use] pub fn nonce(&self) -> u32 { self.params.nonce } + #[must_use] pub fn chain_id(&self) -> &str { &self.params.chain_id } @@ -426,6 +428,7 @@ impl TransactionParamsBuilder { } impl TransactionParamsBuilder { + #[must_use = "the transaction params builder must be built to be useful"] pub fn chain_id<'a, T: Into>>( self, chain_id: T, @@ -436,6 +439,7 @@ impl TransactionParamsBuilder { } } + #[must_use = "the transaction params builder must be built to be useful"] pub fn nonce(self, nonce: u32) -> Self { Self { nonce, @@ -445,19 +449,19 @@ impl TransactionParamsBuilder { } impl<'a> TransactionParamsBuilder> { + /// Constructs a [`TransactionParams`] from the configured builder. + /// + /// # Errors + /// Returns an error if the set chain ID does not contain a chain name that can be turned into + /// a bech32 human readable prefix (everything before the first dash i.e. `-`). pub fn try_build(self) -> Result { let Self { nonce, chain_id, } = self; let chain_id = chain_id.as_ref().trim().to_string(); - let hrp = bech32::Hrp::parse( - chain_id - .split_once('-') - .map(|tup| tup.0) - .unwrap_or(&chain_id), - ) - .map_err(TransactionParamsError::chain_id_not_bech32_compatible)?; + let hrp = bech32::Hrp::parse(chain_id.split_once('-').map_or(&chain_id, |tup| tup.0)) + .map_err(TransactionParamsError::chain_id_not_bech32_compatible)?; Ok(TransactionParams { nonce, chain_id, @@ -474,6 +478,7 @@ pub struct TransactionParams { } impl TransactionParams { + #[must_use = "the transaction params builder must be built to be useful"] pub fn builder() -> TransactionParamsBuilder { TransactionParamsBuilder::new() } @@ -492,6 +497,9 @@ impl TransactionParams { } /// Convert from a raw protobuf [`raw::UnsignedTransaction`]. + /// + /// # Errors + /// See [`TransactionParamsBuilder::try_build`] for errors returned by this method. pub fn try_from_raw(proto: raw::TransactionParams) -> Result { let raw::TransactionParams { nonce,