From 5e3a8a88e75e2ed3d13a95555e2ce671d7843086 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 3 Oct 2023 10:18:04 +0900 Subject: [PATCH 01/10] Enable frozen_abi on banking trace file --- Cargo.lock | 3 +++ core/src/banking_trace.rs | 7 +++--- core/src/sigverify.rs | 2 +- frozen-abi/src/abi_example.rs | 41 +++++++++++++++++++---------------- perf/Cargo.toml | 5 +++++ perf/build.rs | 26 ++++++++++++++++++++++ perf/src/cuda_runtime.rs | 2 +- perf/src/lib.rs | 4 ++++ perf/src/packet.rs | 2 +- perf/src/recycler.rs | 9 ++++++++ programs/sbf/Cargo.lock | 3 +++ sdk/src/packet.rs | 11 ++++++++-- 12 files changed, 88 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b5394925adefb4..31a8083b0469eb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6434,7 +6434,10 @@ dependencies = [ "rand 0.8.5", "rand_chacha 0.3.1", "rayon", + "rustc_version 0.4.0", "serde", + "solana-frozen-abi", + "solana-frozen-abi-macro", "solana-logger", "solana-metrics", "solana-rayon-threadlimit", diff --git a/core/src/banking_trace.rs b/core/src/banking_trace.rs index 760121dc7c557d..ba76b794ba2919 100644 --- a/core/src/banking_trace.rs +++ b/core/src/banking_trace.rs @@ -62,16 +62,17 @@ pub struct BankingTracer { active_tracer: Option, } -#[derive(Serialize, Deserialize, Debug)] +#[frozen_abi(digest = "Eq6YrAFtTbtPrCEvh6Et1mZZDCARUg1gcK2qiZdqyjUz")] +#[derive(Serialize, Deserialize, Debug, AbiExample)] pub struct TimedTracedEvent(pub std::time::SystemTime, pub TracedEvent); -#[derive(Serialize, Deserialize, Debug)] +#[derive(Serialize, Deserialize, Debug, AbiExample, AbiEnumVisitor)] pub enum TracedEvent { PacketBatch(ChannelLabel, BankingPacketBatch), BlockAndBankHash(Slot, Hash, Hash), } -#[derive(Serialize, Deserialize, Debug, Clone, Copy)] +#[derive(Serialize, Deserialize, Debug, Clone, Copy, AbiExample, AbiEnumVisitor)] pub enum ChannelLabel { NonVote, TpuVote, diff --git a/core/src/sigverify.rs b/core/src/sigverify.rs index 8140efac7ec2aa..b496452078d883 100644 --- a/core/src/sigverify.rs +++ b/core/src/sigverify.rs @@ -16,7 +16,7 @@ use { solana_sdk::{packet::Packet, saturating_add_assign}, }; -#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize, AbiExample)] pub struct SigverifyTracerPacketStats { pub total_removed_before_sigverify_stage: usize, pub total_tracer_packets_received_in_sigverify_stage: usize, diff --git a/frozen-abi/src/abi_example.rs b/frozen-abi/src/abi_example.rs index c7765c4a573544..86144774eabcf2 100644 --- a/frozen-abi/src/abi_example.rs +++ b/frozen-abi/src/abi_example.rs @@ -137,25 +137,12 @@ tuple_example_impls! { } } -// Source: https://github.com/rust-lang/rust/blob/ba18875557aabffe386a2534a1aa6118efb6ab88/src/libcore/array/mod.rs#L417 -macro_rules! array_example_impls { - {$n:expr, $t:ident $($ts:ident)*} => { - impl AbiExample for [T; $n] where T: AbiExample { - fn example() -> Self { - [$t::example(), $($ts::example()),*] - } - } - array_example_impls!{($n - 1), $($ts)*} - }; - {$n:expr,} => { - impl AbiExample for [T; $n] { - fn example() -> Self { [] } - } - }; +impl AbiExample for [T; N] { + fn example() -> Self { + std::array::from_fn(|_| T::example()) + } } -array_example_impls! {32, T T T T T T T T T T T T T T T T T T T T T T T T T T T T T T T T} - // Source: https://github.com/rust-lang/rust/blob/ba18875557aabffe386a2534a1aa6118efb6ab88/src/libcore/default.rs#L137 macro_rules! example_impls { ($t:ty, $v:expr) => { @@ -241,6 +228,7 @@ pub(crate) fn normalize_type_name(type_name: &str) -> String { type Placeholder = (); impl AbiExample for T { + #[track_caller] default fn example() -> Self { ::type_erased_example() } @@ -252,6 +240,7 @@ trait TypeErasedExample { } impl TypeErasedExample for Placeholder { + #[track_caller] default fn type_erased_example() -> T { panic!( "derive or implement AbiExample/AbiEnumVisitor for {}", @@ -261,6 +250,7 @@ impl TypeErasedExample for Placeholder { } impl TypeErasedExample for Placeholder { + #[track_caller] default fn type_erased_example() -> T { let original_type_name = type_name::(); let normalized_type_name = normalize_type_name(original_type_name); @@ -329,6 +319,13 @@ impl AbiExample for std::sync::Arc { } } +impl AbiExample for std::sync::Weak { + fn example() -> Self { + info!("AbiExample for (Arc): {}", type_name::()); + std::sync::Arc::downgrade(&std::sync::Arc::new(T::example())) + } +} + impl AbiExample for std::rc::Rc { fn example() -> Self { info!("AbiExample for (Rc): {}", type_name::()); @@ -457,6 +454,13 @@ impl AbiExample for std::path::PathBuf { } } +#[cfg(not(target_os = "solana"))] +impl AbiExample for std::time::SystemTime { + fn example() -> Self { + std::time::SystemTime::now() + } +} + use std::net::{IpAddr, Ipv4Addr, SocketAddr}; impl AbiExample for SocketAddr { fn example() -> Self { @@ -490,8 +494,7 @@ impl AbiEnumVisitor for T { impl AbiEnumVisitor for T { default fn visit_for_abi(&self, digester: &mut AbiDigester) -> DigestResult { info!("AbiEnumVisitor for (default): {}", type_name::()); - T::example() - .serialize(digester.create_new()) + self.serialize(digester.create_new()) .map_err(DigestError::wrap_by_type::) } } diff --git a/perf/Cargo.toml b/perf/Cargo.toml index aea478da078c35..b62484f4249abd 100644 --- a/perf/Cargo.toml +++ b/perf/Cargo.toml @@ -21,6 +21,8 @@ log = { workspace = true } rand = { workspace = true } rayon = { workspace = true } serde = { workspace = true } +solana-frozen-abi = { workspace = true } +solana-frozen-abi-macro = { workspace = true } solana-metrics = { workspace = true } solana-rayon-threadlimit = { workspace = true } solana-sdk = { workspace = true } @@ -40,6 +42,9 @@ rand_chacha = { workspace = true } solana-logger = { workspace = true } test-case = { workspace = true } +[build-dependencies] +rustc_version = { workspace = true } + [[bench]] name = "sigverify" diff --git a/perf/build.rs b/perf/build.rs index 025c71008f092b..4925ee898eb612 100644 --- a/perf/build.rs +++ b/perf/build.rs @@ -1,3 +1,6 @@ +extern crate rustc_version; +use rustc_version::{version_meta, Channel}; + fn main() { #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] { @@ -8,4 +11,27 @@ fn main() { println!("cargo:rustc-cfg=build_target_feature_avx2"); } } + + // Copied and adapted from + // https://github.com/Kimundi/rustc-version-rs/blob/1d692a965f4e48a8cb72e82cda953107c0d22f47/README.md#example + // Licensed under Apache-2.0 + MIT + match version_meta().unwrap().channel { + Channel::Stable => { + println!("cargo:rustc-cfg=RUSTC_WITHOUT_SPECIALIZATION"); + } + Channel::Beta => { + println!("cargo:rustc-cfg=RUSTC_WITHOUT_SPECIALIZATION"); + } + Channel::Nightly => { + println!("cargo:rustc-cfg=RUSTC_WITH_SPECIALIZATION"); + } + Channel::Dev => { + println!("cargo:rustc-cfg=RUSTC_WITH_SPECIALIZATION"); + // See https://github.com/solana-labs/solana/issues/11055 + // We may be running the custom `rust-bpf-builder` toolchain, + // which currently needs `#![feature(proc_macro_hygiene)]` to + // be applied. + println!("cargo:rustc-cfg=RUSTC_NEEDS_PROC_MACRO_HYGIENE"); + } + } } diff --git a/perf/src/cuda_runtime.rs b/perf/src/cuda_runtime.rs index a2986af1813680..5b44099aecb36c 100644 --- a/perf/src/cuda_runtime.rs +++ b/perf/src/cuda_runtime.rs @@ -54,7 +54,7 @@ fn unpin(mem: *mut T) { // A vector wrapper where the underlying memory can be // page-pinned. Controlled by flags in case user only wants // to pin in certain circumstances. -#[derive(Debug, Default, Serialize, Deserialize)] +#[derive(Debug, Default, Serialize, Deserialize, AbiExample)] pub struct PinnedVec { x: Vec, pinned: bool, diff --git a/perf/src/lib.rs b/perf/src/lib.rs index 8d277d7ad69778..83cefe1f319145 100644 --- a/perf/src/lib.rs +++ b/perf/src/lib.rs @@ -1,3 +1,4 @@ +#![cfg_attr(RUSTC_WITH_SPECIALIZATION, feature(min_specialization))] pub mod cuda_runtime; pub mod data_budget; pub mod deduper; @@ -23,6 +24,9 @@ extern crate assert_matches; #[macro_use] extern crate solana_metrics; +#[macro_use] +extern crate solana_frozen_abi_macro; + fn is_rosetta_emulated() -> bool { #[cfg(target_os = "macos")] { diff --git a/perf/src/packet.rs b/perf/src/packet.rs index b030f04dae8ce6..fbb8a437d6bd32 100644 --- a/perf/src/packet.rs +++ b/perf/src/packet.rs @@ -18,7 +18,7 @@ pub const NUM_PACKETS: usize = 1024 * 8; pub const PACKETS_PER_BATCH: usize = 64; pub const NUM_RCVMMSGS: usize = 64; -#[derive(Debug, Default, Clone, Serialize, Deserialize)] +#[derive(Debug, Default, Clone, Serialize, Deserialize, AbiExample)] pub struct PacketBatch { packets: PinnedVec, } diff --git a/perf/src/recycler.rs b/perf/src/recycler.rs index 27c47d0df45103..87c44399e7fbc8 100644 --- a/perf/src/recycler.rs +++ b/perf/src/recycler.rs @@ -57,6 +57,15 @@ impl Default for RecyclerX { } } +#[cfg(RUSTC_WITH_SPECIALIZATION)] +impl solana_frozen_abi::abi_example::AbiExample + for RecyclerX> +{ + fn example() -> Self { + Self::default() + } +} + pub trait Reset { fn reset(&mut self); fn warm(&mut self, size_hint: usize); diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index 3e749d42692ff0..e052f9ffcd6ee2 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -5194,7 +5194,10 @@ dependencies = [ "nix", "rand 0.8.5", "rayon", + "rustc_version", "serde", + "solana-frozen-abi", + "solana-frozen-abi-macro", "solana-metrics", "solana-rayon-threadlimit", "solana-sdk", diff --git a/sdk/src/packet.rs b/sdk/src/packet.rs index b70d8adae8a4bb..b1589ecc4fce8d 100644 --- a/sdk/src/packet.rs +++ b/sdk/src/packet.rs @@ -36,7 +36,7 @@ bitflags! { } } -#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, AbiExample)] #[repr(C)] pub struct Meta { pub size: usize, @@ -45,6 +45,13 @@ pub struct Meta { pub flags: PacketFlags, } +#[cfg(RUSTC_WITH_SPECIALIZATION)] +impl ::solana_frozen_abi::abi_example::AbiExample for PacketFlags { + fn example() -> Self { + Self::empty() + } +} + // serde_as is used as a work around because array isn't supported by serde // (and serde_bytes). // @@ -71,7 +78,7 @@ pub struct Meta { // ryoqun's dirty experiments: // https://github.com/ryoqun/serde-array-comparisons #[serde_as] -#[derive(Clone, Eq, Serialize, Deserialize)] +#[derive(Clone, Eq, Serialize, Deserialize, AbiExample)] #[repr(C)] pub struct Packet { // Bytes past Packet.meta.size are not valid to read from. From c875a1a36fa9e74414958bfc9c3860ed72e1239b Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 3 Oct 2023 22:08:53 +0900 Subject: [PATCH 02/10] Fix ci with really correct bugfix... --- frozen-abi/src/abi_digester.rs | 6 +++--- frozen-abi/src/abi_example.rs | 27 ++++++++++++++++++++------- sdk/src/packet.rs | 8 ++++++++ 3 files changed, 31 insertions(+), 10 deletions(-) diff --git a/frozen-abi/src/abi_digester.rs b/frozen-abi/src/abi_digester.rs index 0d0886daae7438..ad8dcf271af347 100644 --- a/frozen-abi/src/abi_digester.rs +++ b/frozen-abi/src/abi_digester.rs @@ -85,12 +85,12 @@ impl AbiDigester { } } - pub fn create_new_opaque(&self, top_scope: &str) -> Self { + pub fn create_new_opaque(&self, scope: &str) -> Self { Self { data_types: self.data_types.clone(), depth: self.depth, for_enum: false, - opaque_scope: Some(top_scope.to_owned()), + opaque_scope: Some(scope.to_owned()), } } @@ -124,7 +124,7 @@ impl AbiDigester { let type_name = normalize_type_name(type_name::()); if type_name.ends_with("__SerializeWith") || (self.opaque_scope.is_some() - && type_name.starts_with(self.opaque_scope.as_ref().unwrap())) + && type_name.contains(self.opaque_scope.as_ref().unwrap())) { // we can't use the AbiEnumVisitor trait for these cases. value.serialize(self.create_new()) diff --git a/frozen-abi/src/abi_example.rs b/frozen-abi/src/abi_example.rs index 86144774eabcf2..a1f110b59f023a 100644 --- a/frozen-abi/src/abi_example.rs +++ b/frozen-abi/src/abi_example.rs @@ -480,7 +480,9 @@ pub trait AbiEnumVisitor: Serialize { } pub trait IgnoreAsHelper {} -pub trait EvenAsOpaque {} +pub trait EvenAsOpaque { + const SCOPE: Option<&'static str> = None; +} impl AbiEnumVisitor for T { default fn visit_for_abi(&self, _digester: &mut AbiDigester) -> DigestResult { @@ -493,8 +495,11 @@ impl AbiEnumVisitor for T { impl AbiEnumVisitor for T { default fn visit_for_abi(&self, digester: &mut AbiDigester) -> DigestResult { - info!("AbiEnumVisitor for (default): {}", type_name::()); - self.serialize(digester.create_new()) + info!("AbiEnumVisitor for T: {}", type_name::()); + // not calling self.serialize(...) is intentional here as the most generic impl + // consider IgnoreAsHelper and EvenAsOpaque if you're stuck on this.... + T::example() + .serialize(digester.create_new()) .map_err(DigestError::wrap_by_type::) } } @@ -504,7 +509,7 @@ impl AbiEnumVisitor for T { // relevant test: TestVecEnum impl AbiEnumVisitor for &T { default fn visit_for_abi(&self, digester: &mut AbiDigester) -> DigestResult { - info!("AbiEnumVisitor for (&default): {}", type_name::()); + info!("AbiEnumVisitor for &T: {}", type_name::()); // Don't call self.visit_for_abi(...) to avoid the infinite recursion! T::visit_for_abi(self, digester) } @@ -524,9 +529,17 @@ impl AbiEnumVisitor for &T { // inability of implementing AbiExample for private structs from other crates impl AbiEnumVisitor for &T { default fn visit_for_abi(&self, digester: &mut AbiDigester) -> DigestResult { - info!("AbiEnumVisitor for (IgnoreAsOpaque): {}", type_name::()); - let top_scope = type_name::().split("::").next().unwrap(); - self.serialize(digester.create_new_opaque(top_scope)) + let type_name = type_name::(); + let scope = if let Some(scope) = T::SCOPE { + scope + } else { + type_name.split("::").next().unwrap() + }; + info!( + "AbiEnumVisitor for (EvenAsOpaque): {}: scope: {}", + type_name, scope + ); + self.serialize(digester.create_new_opaque(scope)) .map_err(DigestError::wrap_by_type::) } } diff --git a/sdk/src/packet.rs b/sdk/src/packet.rs index b1589ecc4fce8d..cf2f61de149d3b 100644 --- a/sdk/src/packet.rs +++ b/sdk/src/packet.rs @@ -52,6 +52,14 @@ impl ::solana_frozen_abi::abi_example::AbiExample for PacketFlags { } } +#[cfg(RUSTC_WITH_SPECIALIZATION)] +impl ::solana_frozen_abi::abi_example::IgnoreAsHelper for PacketFlags {} + +#[cfg(RUSTC_WITH_SPECIALIZATION)] +impl ::solana_frozen_abi::abi_example::EvenAsOpaque for PacketFlags { + const SCOPE: Option<&'static str> = Some("InternalBitFlags"); +} + // serde_as is used as a work around because array isn't supported by serde // (and serde_bytes). // From 4a34a58ca78c9740d3da7c8623633a86748c7f3d Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 3 Oct 2023 23:03:33 +0900 Subject: [PATCH 03/10] Remove tracker_callers --- frozen-abi/src/abi_example.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/frozen-abi/src/abi_example.rs b/frozen-abi/src/abi_example.rs index a1f110b59f023a..044c05b8a5b1db 100644 --- a/frozen-abi/src/abi_example.rs +++ b/frozen-abi/src/abi_example.rs @@ -228,7 +228,6 @@ pub(crate) fn normalize_type_name(type_name: &str) -> String { type Placeholder = (); impl AbiExample for T { - #[track_caller] default fn example() -> Self { ::type_erased_example() } @@ -240,7 +239,6 @@ trait TypeErasedExample { } impl TypeErasedExample for Placeholder { - #[track_caller] default fn type_erased_example() -> T { panic!( "derive or implement AbiExample/AbiEnumVisitor for {}", @@ -250,7 +248,6 @@ impl TypeErasedExample for Placeholder { } impl TypeErasedExample for Placeholder { - #[track_caller] default fn type_erased_example() -> T { let original_type_name = type_name::(); let normalized_type_name = normalize_type_name(original_type_name); From 368d5a5c921720977fd1e8fe3ee802024f400ce3 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 3 Oct 2023 23:28:54 +0900 Subject: [PATCH 04/10] Fix typo... --- frozen-abi/src/abi_example.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frozen-abi/src/abi_example.rs b/frozen-abi/src/abi_example.rs index 044c05b8a5b1db..01674603fcff17 100644 --- a/frozen-abi/src/abi_example.rs +++ b/frozen-abi/src/abi_example.rs @@ -318,7 +318,7 @@ impl AbiExample for std::sync::Arc { impl AbiExample for std::sync::Weak { fn example() -> Self { - info!("AbiExample for (Arc): {}", type_name::()); + info!("AbiExample for (Weak): {}", type_name::()); std::sync::Arc::downgrade(&std::sync::Arc::new(T::example())) } } From f5bdb30fe9d52baa95190a38e2f10252534a65d6 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Tue, 3 Oct 2023 23:30:40 +0900 Subject: [PATCH 05/10] Fix AbiExample for Arc/Rc's Weaks --- frozen-abi/src/abi_digester.rs | 8 ++++++++ frozen-abi/src/abi_example.rs | 22 ++++++++++++++++++++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/frozen-abi/src/abi_digester.rs b/frozen-abi/src/abi_digester.rs index ad8dcf271af347..90e63c3eed88ed 100644 --- a/frozen-abi/src/abi_digester.rs +++ b/frozen-abi/src/abi_digester.rs @@ -691,4 +691,12 @@ mod tests { Variant2(u8, u16, #[serde(skip)] u32), } } + + #[frozen_abi(digest = "B1PcwZdUfGnxaRid9e6ZwkST3NZ2KUEYobA1DkxWrYLP")] + #[derive(Serialize, AbiExample)] + struct TestArcWeak(std::sync::Weak); + + #[frozen_abi(digest = "4R8uCLR1BVU1aFgkSaNyKcFD1FeM6rGdsjbJBFpnqx4v")] + #[derive(Serialize, AbiExample)] + struct TestRcWeak(std::rc::Weak); } diff --git a/frozen-abi/src/abi_example.rs b/frozen-abi/src/abi_example.rs index 01674603fcff17..2ba53cbee4c99e 100644 --- a/frozen-abi/src/abi_example.rs +++ b/frozen-abi/src/abi_example.rs @@ -316,10 +316,20 @@ impl AbiExample for std::sync::Arc { } } +// When T is weakly owned by the likes of `std::{sync, rc}::Weak`s, we need to uphold the ownership +// of T in some way at least during abi digesting... However, there's no easy way. Stashing them +// into static is confronted with Send/Sync issue. Stashing them into thread_local is confronted +// with not enough (T + 'static) lifetime bound.. So, just leak the examples. This should be +// tolerated, considering ::example() should ever be called inside tests, not in production code... +fn leak_and_inhibit_drop<'a, T>(t: T) -> &'a mut T { + Box::leak(Box::new(t)) +} + impl AbiExample for std::sync::Weak { fn example() -> Self { - info!("AbiExample for (Weak): {}", type_name::()); - std::sync::Arc::downgrade(&std::sync::Arc::new(T::example())) + info!("AbiExample for (Arc's Weak): {}", type_name::()); + // leaking is needed otherwise Arc::upgrade() will always return None... + std::sync::Arc::downgrade(leak_and_inhibit_drop(std::sync::Arc::new(T::example()))) } } @@ -330,6 +340,14 @@ impl AbiExample for std::rc::Rc { } } +impl AbiExample for std::rc::Weak { + fn example() -> Self { + info!("AbiExample for (Rc's Weak): {}", type_name::()); + // leaking is needed otherwise Rc::upgrade() will always return None... + std::rc::Rc::downgrade(leak_and_inhibit_drop(std::rc::Rc::new(T::example()))) + } +} + impl AbiExample for std::sync::Mutex { fn example() -> Self { info!("AbiExample for (Mutex): {}", type_name::()); From 56111b706f0c9e39fad1cc4dacbefb3ced538608 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Wed, 4 Oct 2023 11:32:39 +0900 Subject: [PATCH 06/10] Added comment for AbiExample impl of SystemTime --- frozen-abi/src/abi_example.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/frozen-abi/src/abi_example.rs b/frozen-abi/src/abi_example.rs index 2ba53cbee4c99e..c3552e4e121ce8 100644 --- a/frozen-abi/src/abi_example.rs +++ b/frozen-abi/src/abi_example.rs @@ -472,6 +472,11 @@ impl AbiExample for std::path::PathBuf { #[cfg(not(target_os = "solana"))] impl AbiExample for std::time::SystemTime { fn example() -> Self { + // Returning ever-changing value here will still produce consistent digest, because ABI + // digest only cares about the type hierarchy of construsted object graph of the ABI + // example. It doesn't care about the actual values/contents of the graph not much, because + // it's not relevant for the _interface_ of given binary data. + // That said, ::now() is the only way to construct SystemTime safely. std::time::SystemTime::now() } } From 2230884c3f62d7d52e7ab8d977e38a8462176806 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Wed, 4 Oct 2023 11:54:09 +0900 Subject: [PATCH 07/10] Simplify and document EvenAsOpaque with new usage --- Cargo.lock | 1 + frozen-abi/Cargo.toml | 1 + frozen-abi/src/abi_digester.rs | 45 ++++++++++++++++++++++++++++------ frozen-abi/src/abi_example.rs | 32 +++++++++++++++--------- sdk/src/packet.rs | 2 +- 5 files changed, 61 insertions(+), 20 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 31a8083b0469eb..8e742192dbe5f3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5931,6 +5931,7 @@ dependencies = [ name = "solana-frozen-abi" version = "1.18.0" dependencies = [ + "bitflags 2.3.3", "block-buffer 0.10.4", "bs58", "bv", diff --git a/frozen-abi/Cargo.toml b/frozen-abi/Cargo.toml index 2965dd17a368d7..4a4029ceb843d4 100644 --- a/frozen-abi/Cargo.toml +++ b/frozen-abi/Cargo.toml @@ -31,6 +31,7 @@ subtle = { workspace = true } [target.'cfg(not(target_os = "solana"))'.dev-dependencies] solana-logger = { workspace = true } +bitflags = { workspace = true } [build-dependencies] rustc_version = { workspace = true } diff --git a/frozen-abi/src/abi_digester.rs b/frozen-abi/src/abi_digester.rs index 90e63c3eed88ed..d1d3c61682fb1a 100644 --- a/frozen-abi/src/abi_digester.rs +++ b/frozen-abi/src/abi_digester.rs @@ -17,7 +17,7 @@ pub struct AbiDigester { data_types: std::rc::Rc>>, depth: usize, for_enum: bool, - opaque_scope: Option, + opaque_type_matcher: Option, } pub type DigestResult = Result; @@ -70,7 +70,7 @@ impl AbiDigester { data_types: std::rc::Rc::new(std::cell::RefCell::new(vec![])), for_enum: false, depth: 0, - opaque_scope: None, + opaque_type_matcher: None, } } @@ -81,7 +81,7 @@ impl AbiDigester { data_types: self.data_types.clone(), depth: self.depth, for_enum: false, - opaque_scope: self.opaque_scope.clone(), + opaque_type_matcher: self.opaque_type_matcher.clone(), } } @@ -90,7 +90,7 @@ impl AbiDigester { data_types: self.data_types.clone(), depth: self.depth, for_enum: false, - opaque_scope: Some(scope.to_owned()), + opaque_type_matcher: Some(scope.to_owned()), } } @@ -103,7 +103,7 @@ impl AbiDigester { data_types: self.data_types.clone(), depth, for_enum: false, - opaque_scope: self.opaque_scope.clone(), + opaque_type_matcher: self.opaque_type_matcher.clone(), }) } @@ -116,15 +116,15 @@ impl AbiDigester { data_types: self.data_types.clone(), depth, for_enum: true, - opaque_scope: self.opaque_scope.clone(), + opaque_type_matcher: self.opaque_type_matcher.clone(), }) } pub fn digest_data(&mut self, value: &T) -> DigestResult { let type_name = normalize_type_name(type_name::()); if type_name.ends_with("__SerializeWith") - || (self.opaque_scope.is_some() - && type_name.contains(self.opaque_scope.as_ref().unwrap())) + || (self.opaque_type_matcher.is_some() + && type_name.contains(self.opaque_type_matcher.as_ref().unwrap())) { // we can't use the AbiEnumVisitor trait for these cases. value.serialize(self.create_new()) @@ -661,6 +661,35 @@ mod tests { #[frozen_abi(digest = "9PMdHRb49BpkywrmPoJyZWMsEmf5E1xgmsFGkGmea5RW")] type TestBitVec = bv::BitVec; + mod bitflags { + use crate::abi_example::{AbiExample, EvenAsOpaque, IgnoreAsHelper}; + + bitflags::bitflags! { + #[frozen_abi(digest = "HYjQ8oiYirTnQoLeRHbq66i8VYfcxPA4QTT3gdUQBgyV")] + #[derive(Serialize, Deserialize)] + struct TestFlags: u8 { + const TestBit = 0b0000_0001; + } + } + + impl AbiExample for TestFlags { + fn example() -> Self { + Self::empty() + } + } + + impl IgnoreAsHelper for TestFlags {} + // This (EvenAsOpaque) marker trait is needed for bitflags-generated types because we can't + // impl AbiExample for its + // private type: + // thread '...TestFlags_frozen_abi...' panicked at ...: + // derive or implement AbiExample/AbiEnumVisitor for + // solana_frozen_abi::abi_digester::tests::_::InternalBitFlags + impl EvenAsOpaque for TestFlags { + const TYPE_NAME_MATCHER: &'static str = "::_::InternalBitFlags"; + } + } + mod skip_should_be_same { #[frozen_abi(digest = "4LbuvQLX78XPbm4hqqZcHFHpseDJcw4qZL9EUZXSi2Ss")] #[derive(Serialize, AbiExample)] diff --git a/frozen-abi/src/abi_example.rs b/frozen-abi/src/abi_example.rs index c3552e4e121ce8..760c6571d43668 100644 --- a/frozen-abi/src/abi_example.rs +++ b/frozen-abi/src/abi_example.rs @@ -219,7 +219,14 @@ impl AbiExample for BitVec { } impl IgnoreAsHelper for BitVec {} -impl EvenAsOpaque for BitVec {} +// This (EvenAsOpaque) marker trait is needed for BitVec because we can't impl AbiExample for its +// private type: +// thread '...TestBitVec_frozen_abi...' panicked at ...: +// derive or implement AbiExample/AbiEnumVisitor for +// bv::bit_vec::inner::Inner +impl EvenAsOpaque for BitVec { + const TYPE_NAME_MATCHER: &'static str = "bv::bit_vec::inner::"; +} pub(crate) fn normalize_type_name(type_name: &str) -> String { type_name.chars().filter(|c| *c != '&').collect() @@ -494,14 +501,21 @@ impl AbiExample for IpAddr { } } -// This is a control flow indirection needed for digesting all variants of an enum +// This is a control flow indirection needed for digesting all variants of an enum. +// +// All of types (including non-enums) will be processed by this trait, albeit the +// name of this trait. +// User-defined enums usually just need to impl this with namesake derive macro (AbiEnumVisitor). +// +// Note that sometimes this indirection doesn't work for various reasons. For that end, there are +// hacks with marker traits (IgnoreAsHelper/EvenAsOpaque). pub trait AbiEnumVisitor: Serialize { fn visit_for_abi(&self, digester: &mut AbiDigester) -> DigestResult; } pub trait IgnoreAsHelper {} pub trait EvenAsOpaque { - const SCOPE: Option<&'static str> = None; + const TYPE_NAME_MATCHER: &'static str; } impl AbiEnumVisitor for T { @@ -550,16 +564,12 @@ impl AbiEnumVisitor for &T { impl AbiEnumVisitor for &T { default fn visit_for_abi(&self, digester: &mut AbiDigester) -> DigestResult { let type_name = type_name::(); - let scope = if let Some(scope) = T::SCOPE { - scope - } else { - type_name.split("::").next().unwrap() - }; + let matcher = T::TYPE_NAME_MATCHER; info!( - "AbiEnumVisitor for (EvenAsOpaque): {}: scope: {}", - type_name, scope + "AbiEnumVisitor for (EvenAsOpaque): {}: matcher: {}", + type_name, matcher ); - self.serialize(digester.create_new_opaque(scope)) + self.serialize(digester.create_new_opaque(matcher)) .map_err(DigestError::wrap_by_type::) } } diff --git a/sdk/src/packet.rs b/sdk/src/packet.rs index cf2f61de149d3b..faea9ab4753c67 100644 --- a/sdk/src/packet.rs +++ b/sdk/src/packet.rs @@ -57,7 +57,7 @@ impl ::solana_frozen_abi::abi_example::IgnoreAsHelper for PacketFlags {} #[cfg(RUSTC_WITH_SPECIALIZATION)] impl ::solana_frozen_abi::abi_example::EvenAsOpaque for PacketFlags { - const SCOPE: Option<&'static str> = Some("InternalBitFlags"); + const TYPE_NAME_MATCHER: &'static str = "::_::InternalBitFlags"; } // serde_as is used as a work around because array isn't supported by serde From bd5e2cc6dcb4a9d4e095fe2dbbdb23afc3429b81 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Wed, 4 Oct 2023 13:34:36 +0900 Subject: [PATCH 08/10] Minor clean-ups --- frozen-abi/src/abi_digester.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/frozen-abi/src/abi_digester.rs b/frozen-abi/src/abi_digester.rs index d1d3c61682fb1a..b014efd2ba1570 100644 --- a/frozen-abi/src/abi_digester.rs +++ b/frozen-abi/src/abi_digester.rs @@ -85,12 +85,12 @@ impl AbiDigester { } } - pub fn create_new_opaque(&self, scope: &str) -> Self { + pub fn create_new_opaque(&self, type_matcher: &str) -> Self { Self { data_types: self.data_types.clone(), depth: self.depth, for_enum: false, - opaque_type_matcher: Some(scope.to_owned()), + opaque_type_matcher: Some(type_matcher.to_owned()), } } @@ -661,11 +661,11 @@ mod tests { #[frozen_abi(digest = "9PMdHRb49BpkywrmPoJyZWMsEmf5E1xgmsFGkGmea5RW")] type TestBitVec = bv::BitVec; - mod bitflags { + mod bitflags_abi { use crate::abi_example::{AbiExample, EvenAsOpaque, IgnoreAsHelper}; bitflags::bitflags! { - #[frozen_abi(digest = "HYjQ8oiYirTnQoLeRHbq66i8VYfcxPA4QTT3gdUQBgyV")] + #[frozen_abi(digest = "HhKNkaeAd7AohTb8S8sPKjAWwzxWY2DPz5FvkWmx5bSH")] #[derive(Serialize, Deserialize)] struct TestFlags: u8 { const TestBit = 0b0000_0001; @@ -680,8 +680,7 @@ mod tests { impl IgnoreAsHelper for TestFlags {} // This (EvenAsOpaque) marker trait is needed for bitflags-generated types because we can't - // impl AbiExample for its - // private type: + // impl AbiExample for its private type: // thread '...TestFlags_frozen_abi...' panicked at ...: // derive or implement AbiExample/AbiEnumVisitor for // solana_frozen_abi::abi_digester::tests::_::InternalBitFlags From b564f05637ebc1ce32dad844d622c73f56bc9d00 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Thu, 5 Oct 2023 12:19:26 +0900 Subject: [PATCH 09/10] Simplify SystemTime::example() with UNIX_EPOCH... --- frozen-abi/src/abi_example.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/frozen-abi/src/abi_example.rs b/frozen-abi/src/abi_example.rs index 760c6571d43668..804d8a0ab31ba0 100644 --- a/frozen-abi/src/abi_example.rs +++ b/frozen-abi/src/abi_example.rs @@ -479,12 +479,7 @@ impl AbiExample for std::path::PathBuf { #[cfg(not(target_os = "solana"))] impl AbiExample for std::time::SystemTime { fn example() -> Self { - // Returning ever-changing value here will still produce consistent digest, because ABI - // digest only cares about the type hierarchy of construsted object graph of the ABI - // example. It doesn't care about the actual values/contents of the graph not much, because - // it's not relevant for the _interface_ of given binary data. - // That said, ::now() is the only way to construct SystemTime safely. - std::time::SystemTime::now() + std::time::SystemTime::UNIX_EPOCH } } From fd6c28d7587d925442839f8f3bab8fe131b3edfb Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Fri, 6 Oct 2023 09:39:07 +0900 Subject: [PATCH 10/10] Add comment for AbiExample subtleties --- frozen-abi/src/abi_example.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/frozen-abi/src/abi_example.rs b/frozen-abi/src/abi_example.rs index 804d8a0ab31ba0..ab68c6ff25e32a 100644 --- a/frozen-abi/src/abi_example.rs +++ b/frozen-abi/src/abi_example.rs @@ -6,6 +6,24 @@ use { std::any::type_name, }; +// The most important trait for the abi digesting. This trait is used to create any complexities of +// object graph to generate the abi digest. The frozen abi test harness calls T::example() to +// instantiate the tested root type and traverses its fields recursively, abusing the +// serde::serialize(). +// +// This trait applicability is similar to the Default trait. That means all referenced types must +// implement this trait. AbiExample is implemented for almost all common types in this file. +// +// When implementing AbiExample manually, you need to return a _minimally-populated_ value +// from it to actually generate a meaningful digest. This impl semantics is unlike Default, which +// usually returns something empty. See actual impls for inspiration. +// +// The requirement of AbiExample impls even applies to those types of `#[serde(skip)]`-ed fields. +// That's because the abi digesting needs a properly initialized object to enter into the +// serde::serialize() to begin with, even knowning they aren't used for serialization and thus abi +// digest. Luckily, `#[serde(skip)]`-ed fields' AbiExample impls can just delegate to T::default(), +// exploiting the nature of this artificial impl requirement as an exception from the usual +// AbiExample semantics. pub trait AbiExample: Sized { fn example() -> Self; }