Skip to content

Commit

Permalink
Enable frozen_abi on banking trace file (#33501)
Browse files Browse the repository at this point in the history
* Enable frozen_abi on banking trace file

* Fix ci with really correct bugfix...

* Remove tracker_callers

* Fix typo...

* Fix AbiExample for Arc/Rc's Weaks

* Added comment for AbiExample impl of SystemTime

* Simplify and document EvenAsOpaque with new usage

* Minor clean-ups

* Simplify SystemTime::example() with UNIX_EPOCH...

* Add comment for AbiExample subtleties

(cherry picked from commit 95810d8)

# Conflicts:
#	Cargo.lock
  • Loading branch information
ryoqun authored and mergify[bot] committed Oct 7, 2023
1 parent c534100 commit 05c8f68
Show file tree
Hide file tree
Showing 14 changed files with 208 additions and 42 deletions.
7 changes: 7 additions & 0 deletions Cargo.lock

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

7 changes: 4 additions & 3 deletions core/src/banking_trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,17 @@ pub struct BankingTracer {
active_tracer: Option<ActiveTracer>,
}

#[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,
Expand Down
2 changes: 1 addition & 1 deletion core/src/sigverify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions frozen-abi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ cc = { workspace = true, features = ["jobserver", "parallel"] }

[target.'cfg(not(target_os = "solana"))'.dev-dependencies]
solana-logger = { workspace = true }
bitflags = { workspace = true }

[build-dependencies]
rustc_version = { workspace = true }
54 changes: 45 additions & 9 deletions frozen-abi/src/abi_digester.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub struct AbiDigester {
data_types: std::rc::Rc<std::cell::RefCell<Vec<String>>>,
depth: usize,
for_enum: bool,
opaque_scope: Option<String>,
opaque_type_matcher: Option<String>,
}

pub type DigestResult = Result<AbiDigester, DigestError>;
Expand Down Expand Up @@ -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,
}
}

Expand All @@ -81,16 +81,16 @@ 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(),
}
}

pub fn create_new_opaque(&self, top_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_scope: Some(top_scope.to_owned()),
opaque_type_matcher: Some(type_matcher.to_owned()),
}
}

Expand All @@ -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(),
})
}

Expand All @@ -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<T: ?Sized + Serialize>(&mut self, value: &T) -> DigestResult {
let type_name = normalize_type_name(type_name::<T>());
if type_name.ends_with("__SerializeWith")
|| (self.opaque_scope.is_some()
&& type_name.starts_with(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())
Expand Down Expand Up @@ -661,6 +661,34 @@ mod tests {
#[frozen_abi(digest = "9PMdHRb49BpkywrmPoJyZWMsEmf5E1xgmsFGkGmea5RW")]
type TestBitVec = bv::BitVec<u64>;

mod bitflags_abi {
use crate::abi_example::{AbiExample, EvenAsOpaque, IgnoreAsHelper};

bitflags::bitflags! {
#[frozen_abi(digest = "HhKNkaeAd7AohTb8S8sPKjAWwzxWY2DPz5FvkWmx5bSH")]
#[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)]
Expand Down Expand Up @@ -691,4 +719,12 @@ mod tests {
Variant2(u8, u16, #[serde(skip)] u32),
}
}

#[frozen_abi(digest = "B1PcwZdUfGnxaRid9e6ZwkST3NZ2KUEYobA1DkxWrYLP")]
#[derive(Serialize, AbiExample)]
struct TestArcWeak(std::sync::Weak<u64>);

#[frozen_abi(digest = "4R8uCLR1BVU1aFgkSaNyKcFD1FeM6rGdsjbJBFpnqx4v")]
#[derive(Serialize, AbiExample)]
struct TestRcWeak(std::rc::Weak<u64>);
}
109 changes: 84 additions & 25 deletions frozen-abi/src/abi_example.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -137,25 +155,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<T> AbiExample for [T; $n] where T: AbiExample {
fn example() -> Self {
[$t::example(), $($ts::example()),*]
}
}
array_example_impls!{($n - 1), $($ts)*}
};
{$n:expr,} => {
impl<T> AbiExample for [T; $n] {
fn example() -> Self { [] }
}
};
impl<const N: usize, T: AbiExample> 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) => {
Expand Down Expand Up @@ -232,7 +237,14 @@ impl<T: BlockType> AbiExample for BitVec<T> {
}

impl<T: BlockType> IgnoreAsHelper for BitVec<T> {}
impl<T: BlockType> EvenAsOpaque for BitVec<T> {}
// 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<u64>
impl<T: BlockType> EvenAsOpaque for BitVec<T> {
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()
Expand Down Expand Up @@ -329,13 +341,38 @@ impl<T: AbiExample> AbiExample for std::sync::Arc<T> {
}
}

// 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<T: AbiExample> AbiExample for std::sync::Weak<T> {
fn example() -> Self {
info!("AbiExample for (Arc's Weak<T>): {}", type_name::<Self>());
// leaking is needed otherwise Arc::upgrade() will always return None...
std::sync::Arc::downgrade(leak_and_inhibit_drop(std::sync::Arc::new(T::example())))
}
}

impl<T: AbiExample> AbiExample for std::rc::Rc<T> {
fn example() -> Self {
info!("AbiExample for (Rc<T>): {}", type_name::<Self>());
std::rc::Rc::new(T::example())
}
}

impl<T: AbiExample> AbiExample for std::rc::Weak<T> {
fn example() -> Self {
info!("AbiExample for (Rc's Weak<T>): {}", type_name::<Self>());
// 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<T: AbiExample> AbiExample for std::sync::Mutex<T> {
fn example() -> Self {
info!("AbiExample for (Mutex<T>): {}", type_name::<Self>());
Expand Down Expand Up @@ -457,6 +494,13 @@ impl AbiExample for std::path::PathBuf {
}
}

#[cfg(not(target_os = "solana"))]
impl AbiExample for std::time::SystemTime {
fn example() -> Self {
std::time::SystemTime::UNIX_EPOCH
}
}

use std::net::{IpAddr, Ipv4Addr, SocketAddr};
impl AbiExample for SocketAddr {
fn example() -> Self {
Expand All @@ -470,13 +514,22 @@ 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 {}
pub trait EvenAsOpaque {
const TYPE_NAME_MATCHER: &'static str;
}

impl<T: Serialize + ?Sized> AbiEnumVisitor for T {
default fn visit_for_abi(&self, _digester: &mut AbiDigester) -> DigestResult {
Expand All @@ -489,7 +542,9 @@ impl<T: Serialize + ?Sized> AbiEnumVisitor for T {

impl<T: Serialize + ?Sized + AbiExample> AbiEnumVisitor for T {
default fn visit_for_abi(&self, digester: &mut AbiDigester) -> DigestResult {
info!("AbiEnumVisitor for (default): {}", type_name::<T>());
info!("AbiEnumVisitor for T: {}", type_name::<T>());
// 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::<T>)
Expand All @@ -501,7 +556,7 @@ impl<T: Serialize + ?Sized + AbiExample> AbiEnumVisitor for T {
// relevant test: TestVecEnum
impl<T: Serialize + ?Sized + AbiEnumVisitor> AbiEnumVisitor for &T {
default fn visit_for_abi(&self, digester: &mut AbiDigester) -> DigestResult {
info!("AbiEnumVisitor for (&default): {}", type_name::<T>());
info!("AbiEnumVisitor for &T: {}", type_name::<T>());
// Don't call self.visit_for_abi(...) to avoid the infinite recursion!
T::visit_for_abi(self, digester)
}
Expand All @@ -521,9 +576,13 @@ impl<T: Serialize + IgnoreAsHelper> AbiEnumVisitor for &T {
// inability of implementing AbiExample for private structs from other crates
impl<T: Serialize + IgnoreAsHelper + EvenAsOpaque> AbiEnumVisitor for &T {
default fn visit_for_abi(&self, digester: &mut AbiDigester) -> DigestResult {
info!("AbiEnumVisitor for (IgnoreAsOpaque): {}", type_name::<T>());
let top_scope = type_name::<T>().split("::").next().unwrap();
self.serialize(digester.create_new_opaque(top_scope))
let type_name = type_name::<T>();
let matcher = T::TYPE_NAME_MATCHER;
info!(
"AbiEnumVisitor for (EvenAsOpaque): {}: matcher: {}",
type_name, matcher
);
self.serialize(digester.create_new_opaque(matcher))
.map_err(DigestError::wrap_by_type::<T>)
}
}
Expand Down
5 changes: 5 additions & 0 deletions perf/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -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"

Expand Down
Loading

0 comments on commit 05c8f68

Please sign in to comment.