From 401dfb5de3360a7bfa012f4908560fc936559637 Mon Sep 17 00:00:00 2001 From: Richard Janis Goldschmidt Date: Wed, 20 Nov 2024 15:43:15 +0100 Subject: [PATCH] refactor(core): make crypto module into crate (#1800) ## Summary Breaks out `astria_core::crypto` to crate `astria-core-crypto`. ## Background One needs to import the entire kitchensink that is `astria-core` even if one only wants to use a fraction of its types. That leads to extremely long compilation times because all of its dependencies need to be compiled. This patch is the start to breaking out parts of `astria-core` into their own free standing crates, which `astria-core` then reexports. A useful sideeffect of this change is that the coupling between the different parts of astria-core is reduced (for example, by removing the rarely used utility method `SigningKey::try_address`, allowing to decouple address and crypto logic). ## Changes - Move the module `astria_core::crypto` into new create `astria-core-crypto` and reexport under the `astria_core::crypto` alias (not breaking to consumers) - Remove the tight coupling between the crypto and address parts of the stack by removing the `SigningKey::try_address` (rarely used and only inside a few tests) method and moving `ADDRESS_LENGTH` to a thin `astria-core-consts` crate (this is a breaking change for users of `SigningKey::try_address`). ## Testing Tests for astria core's crypto refinement types were moved to `astria-core-crypto` and left unchanged. Sequencer tests were updated to use `Address::builder` instead of `SigningKey::try_address` and otherwise also left unchanged. ## Changelogs Changelogs updated. ## Breaking Changelist - These changes leave all services untouched. - These changes would be breaking the public API of `astria-core` due to the removal fo `SigningKey::try_address` but we don't provide any stability guarantees for it. ## Related Issues Part of https://github.com/astriaorg/astria/issues/1798 --- Cargo.lock | 19 ++++++++- Cargo.toml | 4 ++ crates/astria-cli/src/sequencer/account.rs | 7 +++- crates/astria-core-consts/Cargo.toml | 6 +++ crates/astria-core-consts/src/lib.rs | 3 ++ crates/astria-core-crypto/CHANGELOG.md | 14 +++++++ crates/astria-core-crypto/Cargo.toml | 22 ++++++++++ .../src/lib.rs} | 41 ++++++------------- crates/astria-core/CHANGELOG.md | 7 ++++ crates/astria-core/Cargo.toml | 10 ++--- crates/astria-core/src/lib.rs | 2 +- crates/astria-core/src/primitive/v1/mod.rs | 3 +- .../src/app/tests_breaking_changes.rs | 23 +++++++++-- .../src/app/tests_execute_transaction.rs | 13 ++++-- 14 files changed, 127 insertions(+), 47 deletions(-) create mode 100644 crates/astria-core-consts/Cargo.toml create mode 100644 crates/astria-core-consts/src/lib.rs create mode 100644 crates/astria-core-crypto/CHANGELOG.md create mode 100644 crates/astria-core-crypto/Cargo.toml rename crates/{astria-core/src/crypto.rs => astria-core-crypto/src/lib.rs} (93%) diff --git a/Cargo.lock b/Cargo.lock index b95f060418..ffcd83f1e0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -701,6 +701,8 @@ name = "astria-core" version = "0.1.0" dependencies = [ "astria-core", + "astria-core-consts", + "astria-core-crypto", "astria-merkle", "base64 0.21.7", "base64-serde", @@ -709,7 +711,6 @@ dependencies = [ "bytes", "celestia-tendermint", "celestia-types", - "ed25519-consensus", "hex", "ibc-types", "indexmap 2.4.0", @@ -728,6 +729,22 @@ dependencies = [ "thiserror", "tonic 0.10.2", "tracing", +] + +[[package]] +name = "astria-core-consts" +version = "0.1.0" + +[[package]] +name = "astria-core-crypto" +version = "0.1.0" +dependencies = [ + "astria-core-consts", + "base64 0.21.7", + "ed25519-consensus", + "rand 0.8.5", + "sha2 0.10.8", + "thiserror", "zeroize", ] diff --git a/Cargo.toml b/Cargo.toml index d1cc181333..ae4a4cda5c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,6 +10,8 @@ members = [ "crates/astria-conductor", "crates/astria-config", "crates/astria-core", + "crates/astria-core-consts", + "crates/astria-core-crypto", "crates/astria-eyre", "crates/astria-grpc-mock", "crates/astria-grpc-mock-test", @@ -34,6 +36,8 @@ default-members = [ "crates/astria-conductor", "crates/astria-config", "crates/astria-core", + "crates/astria-core-consts", + "crates/astria-core-crypto", "crates/astria-grpc-mock", "crates/astria-grpc-mock-test", "crates/astria-grpc-mock-test-codegen", diff --git a/crates/astria-cli/src/sequencer/account.rs b/crates/astria-cli/src/sequencer/account.rs index 269470725f..e6160730f0 100644 --- a/crates/astria-cli/src/sequencer/account.rs +++ b/crates/astria-cli/src/sequencer/account.rs @@ -51,7 +51,12 @@ impl Create { let signing_key = SigningKey::new(OsRng); let pretty_signing_key = hex::encode(signing_key.as_bytes()); let pretty_verifying_key = hex::encode(signing_key.verification_key().as_bytes()); - let pretty_address = SigningKey::try_address(&signing_key, &self.prefix)?; + + let pretty_address: Address = Address::builder() + .array(signing_key.address_bytes()) + .prefix(&self.prefix) + .try_build()?; + println!("Create Sequencer Account"); println!(); // TODO: don't print private keys to CLI, prefer writing to file: diff --git a/crates/astria-core-consts/Cargo.toml b/crates/astria-core-consts/Cargo.toml new file mode 100644 index 0000000000..125b069104 --- /dev/null +++ b/crates/astria-core-consts/Cargo.toml @@ -0,0 +1,6 @@ +[package] +name = "astria-core-consts" +version = "0.1.0" +edition = "2021" + +[dependencies] diff --git a/crates/astria-core-consts/src/lib.rs b/crates/astria-core-consts/src/lib.rs new file mode 100644 index 0000000000..1b05c26f66 --- /dev/null +++ b/crates/astria-core-consts/src/lib.rs @@ -0,0 +1,3 @@ +//! Public constants that are used throughout the Astria stack. + +pub const ADDRESS_LENGTH: usize = 20; diff --git a/crates/astria-core-crypto/CHANGELOG.md b/crates/astria-core-crypto/CHANGELOG.md new file mode 100644 index 0000000000..f782487e36 --- /dev/null +++ b/crates/astria-core-crypto/CHANGELOG.md @@ -0,0 +1,14 @@ + + +# Changelog + +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), +and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## Unreleased + +### Added + +- Initial release [#1800](https://github.com/astriaorg/astria/pull/1800) diff --git a/crates/astria-core-crypto/Cargo.toml b/crates/astria-core-crypto/Cargo.toml new file mode 100644 index 0000000000..4e8b94bf7c --- /dev/null +++ b/crates/astria-core-crypto/Cargo.toml @@ -0,0 +1,22 @@ +[package] +name = "astria-core-crypto" +version = "0.1.0" +edition = "2021" +rust-version = "1.81.0" +license = "MIT OR Apache-2.0" +readme = "README.md" +repository = "https://github.com/astriaorg/astria" +homepage = "https://astria.org" + +[dependencies] +astria-core-consts = { path = "../astria-core-consts" } + +base64 = { workspace = true } +rand = { workspace = true } +sha2 = { workspace = true } +thiserror = { workspace = true } + +ed25519-consensus = { version = "2.1.0", default-features = false, features = [ + "std", +] } +zeroize = { version = "1.7.0", features = ["zeroize_derive"] } diff --git a/crates/astria-core/src/crypto.rs b/crates/astria-core-crypto/src/lib.rs similarity index 93% rename from crates/astria-core/src/crypto.rs rename to crates/astria-core-crypto/src/lib.rs index 2ce819d875..4213a7bdf4 100644 --- a/crates/astria-core/src/crypto.rs +++ b/crates/astria-core-crypto/src/lib.rs @@ -13,6 +13,7 @@ use std::{ sync::OnceLock, }; +pub use astria_core_consts::ADDRESS_LENGTH; use base64::{ display::Base64Display, prelude::BASE64_STANDARD, @@ -28,21 +29,11 @@ use rand::{ CryptoRng, RngCore, }; -use sha2::{ - Digest as _, - Sha256, -}; use zeroize::{ Zeroize, ZeroizeOnDrop, }; -use crate::primitive::v1::{ - Address, - AddressError, - ADDRESS_LEN, -}; - /// An Ed25519 signing key. // *Implementation note*: this is currently a refinement type around // ed25519_consensus::SigningKey overriding its Debug implementation @@ -85,22 +76,9 @@ impl SigningKey { /// Returns the address bytes of the verification key associated with this signing key. #[must_use] - pub fn address_bytes(&self) -> [u8; ADDRESS_LEN] { + pub fn address_bytes(&self) -> [u8; ADDRESS_LENGTH] { *self.verification_key().address_bytes() } - - /// Attempts to create an Astria bech32m `[Address]` with the given prefix. - /// - /// # Errors - /// Returns an [`AddressError`] if an address could not be constructed - /// with the given prefix. Usually if the prefix was too long or contained - /// characters not allowed by bech32m. - pub fn try_address(&self, prefix: &str) -> Result { - Address::builder() - .prefix(prefix) - .array(self.address_bytes()) - .try_build() - } } impl Debug for SigningKey { @@ -139,7 +117,7 @@ pub struct VerificationKey { // The address-bytes are 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_bytes: OnceLock<[u8; ADDRESS_LEN]>, + address_bytes: OnceLock<[u8; ADDRESS_LENGTH]>, } impl VerificationKey { @@ -167,18 +145,23 @@ impl VerificationKey { /// /// The address is the first 20 bytes of the sha256 hash of the verification key. #[must_use] - pub fn address_bytes(&self) -> &[u8; ADDRESS_LEN] { + pub fn address_bytes(&self) -> &[u8; ADDRESS_LENGTH] { + use sha2::{ + Digest as _, + Sha256, + }; + self.address_bytes.get_or_init(|| { - fn first_20(array: [u8; 32]) -> [u8; ADDRESS_LEN] { + fn first_20(array: [u8; 32]) -> [u8; ADDRESS_LENGTH] { [ 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 + /// this ensures that `ADDRESS_LENGTH` is never accidentally changed to a value /// that would violate this assumption. - const _: () = assert!(ADDRESS_LEN <= 32); + const _: () = assert!(ADDRESS_LENGTH <= 32); let bytes: [u8; 32] = Sha256::digest(self).into(); first_20(bytes) }) diff --git a/crates/astria-core/CHANGELOG.md b/crates/astria-core/CHANGELOG.md index dd487eeab3..1d113ed38a 100644 --- a/crates/astria-core/CHANGELOG.md +++ b/crates/astria-core/CHANGELOG.md @@ -15,6 +15,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added method `TracePrefixed::leading_channel` to read the left-most channel of a trace prefixed ICS20 asset [#1768](https://github.com/astriaorg/astria/pull/1768) +### Changed + +- Moved `astria_core::crypto` to `astria-core-crypto` and reexported + `astria_core_crypto as crypto` (this change is transparent) + [#1800](https://github.com/astriaorg/astria/pull/1800/) + ### Removed - Removed method `TracePrefixed::last_channel` [#1768](https://github.com/astriaorg/astria/pull/1768) +- Removed method `SigningKey::try_address` [#1800](https://github.com/astriaorg/astria/pull/1800/) diff --git a/crates/astria-core/Cargo.toml b/crates/astria-core/Cargo.toml index a7a45d5ba0..e8774c9a16 100644 --- a/crates/astria-core/Cargo.toml +++ b/crates/astria-core/Cargo.toml @@ -21,13 +21,12 @@ brotli = { version = "5.0.0", optional = true } celestia-types = { version = "0.1.1", optional = true } pbjson = { version = "0.6.0", optional = true } +astria-core-consts = { path = "../astria-core-consts" } +astria-core-crypto = { path = "../astria-core-crypto" } merkle = { package = "astria-merkle", path = "../astria-merkle" } bytes = { workspace = true } celestia-tendermint = { workspace = true } -ed25519-consensus = { version = "2.1.0", default-features = false, features = [ - "std", -] } hex = { workspace = true } ibc-types = { workspace = true } indexmap = { workspace = true } @@ -35,7 +34,7 @@ pbjson-types = { workspace = true } penumbra-ibc = { workspace = true } penumbra-proto = { workspace = true } prost = { workspace = true } -rand = { workspace = true } +rand = { workspace = true, optional = true } serde = { workspace = true, features = ["derive"], optional = true } sha2 = { workspace = true } tendermint = { workspace = true } @@ -45,14 +44,13 @@ tonic = { workspace = true, optional = true } tracing = { workspace = true } base64-serde = { workspace = true, optional = true } base64 = { workspace = true } -zeroize = { version = "1.7.0", features = ["zeroize_derive"] } [features] celestia = ["dep:celestia-types"] client = ["dep:tonic"] serde = ["dep:serde", "dep:pbjson", "dep:base64-serde"] server = ["dep:tonic"] -test-utils = [] +test-utils = ["dep:rand"] base64-serde = ["dep:base64-serde"] brotli = ["dep:brotli"] # When enabled, this adds constructors for some types that skip the normal constructor validity diff --git a/crates/astria-core/src/lib.rs b/crates/astria-core/src/lib.rs index 5c97446d17..fbf02402dc 100644 --- a/crates/astria-core/src/lib.rs +++ b/crates/astria-core/src/lib.rs @@ -1,3 +1,4 @@ +pub use astria_core_crypto as crypto; use prost::Name; #[cfg(not(target_pointer_width = "64"))] @@ -13,7 +14,6 @@ compile_error!( )] pub mod generated; -pub mod crypto; pub mod execution; pub mod primitive; pub mod protocol; diff --git a/crates/astria-core/src/primitive/v1/mod.rs b/crates/astria-core/src/primitive/v1/mod.rs index 56cb6922b5..bc88b6feed 100644 --- a/crates/astria-core/src/primitive/v1/mod.rs +++ b/crates/astria-core/src/primitive/v1/mod.rs @@ -6,6 +6,7 @@ use std::{ str::FromStr, }; +pub use astria_core_consts::ADDRESS_LENGTH as ADDRESS_LEN; use base64::{ display::Base64Display, prelude::BASE64_URL_SAFE, @@ -21,8 +22,6 @@ use crate::{ Protobuf, }; -pub const ADDRESS_LEN: usize = 20; - pub const ROLLUP_ID_LEN: usize = 32; pub const TRANSACTION_ID_LEN: usize = 32; diff --git a/crates/astria-sequencer/src/app/tests_breaking_changes.rs b/crates/astria-sequencer/src/app/tests_breaking_changes.rs index 031538a057..b70ee03d8f 100644 --- a/crates/astria-sequencer/src/app/tests_breaking_changes.rs +++ b/crates/astria-sequencer/src/app/tests_breaking_changes.rs @@ -15,7 +15,10 @@ use std::{ }; use astria_core::{ - primitive::v1::RollupId, + primitive::v1::{ + Address, + RollupId, + }, protocol::{ genesis::v1::Account, transaction::v1::{ @@ -190,8 +193,22 @@ async fn app_execute_transaction_with_every_action_snapshot() { }; let genesis_state = astria_core::generated::protocol::genesis::v1::GenesisAppState { accounts, - authority_sudo_address: Some(alice.try_address(ASTRIA_PREFIX).unwrap().to_raw()), - ibc_sudo_address: Some(alice.try_address(ASTRIA_PREFIX).unwrap().to_raw()), + authority_sudo_address: Some( + Address::builder() + .prefix(ASTRIA_PREFIX) + .array(alice.address_bytes()) + .try_build() + .unwrap() + .to_raw(), + ), + ibc_sudo_address: Some( + Address::builder() + .prefix(ASTRIA_PREFIX) + .array(alice.address_bytes()) + .try_build() + .unwrap() + .to_raw(), + ), ..proto_genesis_state() } .try_into() diff --git a/crates/astria-sequencer/src/app/tests_execute_transaction.rs b/crates/astria-sequencer/src/app/tests_execute_transaction.rs index 76b1a1ae8e..b4785380b3 100644 --- a/crates/astria-sequencer/src/app/tests_execute_transaction.rs +++ b/crates/astria-sequencer/src/app/tests_execute_transaction.rs @@ -4,6 +4,7 @@ use astria_core::{ crypto::SigningKey, primitive::v1::{ asset, + Address, RollupId, }, protocol::{ @@ -78,14 +79,18 @@ use crate::{ fn proto_genesis_state() -> astria_core::generated::protocol::genesis::v1::GenesisAppState { astria_core::generated::protocol::genesis::v1::GenesisAppState { authority_sudo_address: Some( - get_alice_signing_key() - .try_address(ASTRIA_PREFIX) + Address::builder() + .prefix(ASTRIA_PREFIX) + .array(get_alice_signing_key().address_bytes()) + .try_build() .unwrap() .to_raw(), ), ibc_sudo_address: Some( - get_alice_signing_key() - .try_address(ASTRIA_PREFIX) + Address::builder() + .prefix(ASTRIA_PREFIX) + .array(get_alice_signing_key().address_bytes()) + .try_build() .unwrap() .to_raw(), ),