Skip to content

Commit

Permalink
feat(core, proto)!: add bech32m addresses (#1124)
Browse files Browse the repository at this point in the history
## Summary
Adds bech32m addresses.

## Background
bech32m encoded addresses are the de-facto standard in cosmos. By using
a `"astria"` prefix (in the human readable prefix, bech32 HRP sense) we
align astria with the rest of the cosmos ecosystem.

## Changes
- Add a field `bech32m` to `astria.primitive.v1.Address`
- Deprecate `astria.primitive.v1.Address.inner`
- Change `std::fmt::Display for Address` to use bech32m encoding
- Serialize `Address` in terms of its protobuf-json mapping
- Update the `Address:try_from_raw` constructor to allow ingesting
protobufs that have their `inner` or `bech32m` or populated. `inner` and
`bech32m` are mutually exclusive, only one may be set.
- Protobufs with `inner` set are assumed to have prefix `"astria"`.
- Introduce an `AddressBuilder` type state builder that requires both a
prefix and array/byte slice be set.
- Update all services to the new way of constructing addresses, but with
the prefix `"astria"` set everywhere
- Add a `SignedTransaction::address` method that constructs an address
from the verification key and the `chain_id` stored in the unsigned
transaction's `transaction_params::chain_id`.
- Provide a builder for `TransactionParams` enforcing that chain IDs are
have hrp compatible names.

## Testing
- update snapshot tests for address-as-json serialization
- add unit tests:
  - addresses with only bytes are accepted
  - addresses with only bech32m are accepted
  - addresses with both bytes and bech32m are are rejected
- protobufs created from the common `Address` type only have `bech32m`
populated
- various tests to assert `Address` invariants (mainly maximum length of
prefix + fixed-size address always being encodeable to a string)
- updated all tests that somehow make use of addresses (mainly
sequencer, bridge-withdrawer, composer)

## Breaking Changelist
This patch is marked breaking because the sequencer snapshot tests are
triggered. They all contain actions that themselves contain protobuf
address. This patch is backward but not forward compatible.

## Related Issues
closes #943
  • Loading branch information
SuperFluffy authored Jun 7, 2024
1 parent b52a824 commit ab8705f
Show file tree
Hide file tree
Showing 55 changed files with 1,298 additions and 681 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

40 changes: 32 additions & 8 deletions crates/astria-bridge-withdrawer/src/withdrawer/ethereum/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@ use std::time::Duration;

use astria_core::{
primitive::v1::{
asset,
asset::Denom,
asset::{
self,
Denom,
},
Address,
ASTRIA_ADDRESS_PREFIX,
},
protocol::transaction::v1alpha1::{
action::{
Expand Down Expand Up @@ -94,7 +98,11 @@ fn event_to_bridge_unlock(
transaction_hash,
};
let action = BridgeUnlockAction {
to: event.destination_chain_address.to_fixed_bytes().into(),
to: Address::builder()
.array(event.destination_chain_address.to_fixed_bytes())
.prefix(ASTRIA_ADDRESS_PREFIX)
.try_build()
.wrap_err("failed to construct destination address")?,
amount: event
.amount
.as_u128()
Expand Down Expand Up @@ -126,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
Expand All @@ -147,7 +155,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: sender,
return_address: Address::builder()
.array(sender)
.prefix(ASTRIA_ADDRESS_PREFIX)
.try_build()
.wrap_err("failed to construct return address")?,
amount: event
.amount
.as_u128()
Expand Down Expand Up @@ -202,7 +214,11 @@ mod tests {
};

let expected_action = BridgeUnlockAction {
to: [1u8; 20].into(),
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(),
Expand Down Expand Up @@ -234,7 +250,11 @@ mod tests {
};

let expected_action = BridgeUnlockAction {
to: [1u8; 20].into(),
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(),
Expand Down Expand Up @@ -274,7 +294,11 @@ 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_ADDRESS_PREFIX)
.try_build()
.unwrap(),
amount: 99,
memo: serde_json::to_string(&Ics20WithdrawalMemo {
memo: "hello".to_string(),
Expand Down
16 changes: 10 additions & 6 deletions crates/astria-bridge-withdrawer/src/withdrawer/submitter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()),
)
)]
Expand All @@ -243,7 +247,7 @@ async fn submit_tx(
tx: SignedTransaction,
state: Arc<State>,
) -> eyre::Result<tx_commit::Response> {
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");
Expand Down
16 changes: 13 additions & 3 deletions crates/astria-bridge-withdrawer/src/withdrawer/submitter/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +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,
Expand All @@ -27,7 +33,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_ADDRESS_PREFIX)
.try_build()
.wrap_err("failed to construct Sequencer address")?,
signing_key,
})
}
Expand Down
18 changes: 15 additions & 3 deletions crates/astria-bridge-withdrawer/src/withdrawer/submitter/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -167,7 +171,11 @@ 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)
.try_build()
.unwrap(),
amount: 99,
memo: serde_json::to_string(&Ics20WithdrawalMemo {
memo: "hello".to_string(),
Expand All @@ -187,7 +195,11 @@ 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)
.try_build()
.unwrap(),
amount: 99,
memo: serde_json::to_vec(&BridgeUnlockMemo {
block_number: 1.into(),
Expand Down
5 changes: 2 additions & 3 deletions crates/astria-cli/src/cli/sequencer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
Expand Down Expand Up @@ -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));
Expand Down
14 changes: 7 additions & 7 deletions crates/astria-cli/src/commands/sequencer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -453,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);
Expand Down
27 changes: 27 additions & 0 deletions crates/astria-cli/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,30 @@
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)
.try_build()
.unwrap()
}

/// 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, AddressError> {
Address::builder()
.slice(slice)
.prefix(ADDRESS_PREFIX)
.try_build()
}
10 changes: 9 additions & 1 deletion crates/astria-composer/src/executor/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ use std::{

use astria_core::{
crypto::SigningKey,
primitive::v1::{
Address,
ASTRIA_ADDRESS_PREFIX,
},
protocol::transaction::v1alpha1::action::SequenceAction,
};
use astria_eyre::eyre::{
Expand Down Expand Up @@ -50,7 +54,11 @@ impl Builder {
format!("failed reading signing key from file at path `{private_key_file}`")
})?;

let sequencer_address = *sequencer_key.verification_key().address();
let sequencer_address = Address::builder()
.prefix(ASTRIA_ADDRESS_PREFIX)
.array(sequencer_key.verification_key().address_bytes())
.try_build()
.wrap_err("failed constructing a sequencer address from private key")?;

let (serialized_rollup_transaction_tx, serialized_rollup_transaction_rx) =
tokio::sync::mpsc::channel::<SequenceAction>(256);
Expand Down
22 changes: 12 additions & 10 deletions crates/astria-composer/src/executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,15 +467,15 @@ 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())),
)
)]
async fn submit_tx(
client: sequencer_client::HttpClient,
tx: SignedTransaction,
) -> eyre::Result<tx_sync::Response> {
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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 1 addition & 4 deletions crates/astria-composer/tests/blackbox/helper/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions crates/astria-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
Loading

0 comments on commit ab8705f

Please sign in to comment.