From db4fbefafd1aad503ee9df8a2870b2505d2b7c44 Mon Sep 17 00:00:00 2001 From: Oliver Anyanwu Date: Mon, 13 Jul 2026 12:04:32 +0200 Subject: [PATCH 1/2] Add an IsEmpty derive macro and use it in pos-accounting PoSAccountingData had a hand-written is_empty that ANDs all of its fields, with a TODO asking for it to be generated. It is easy to add a field and forget to update such a method. Add an is-empty crate with an IsEmpty trait and a matching derive, mirroring the existing typename/typename-derive pair. The derive implements is_empty as the conjunction of each field's own is_empty, so a value is empty when all of its fields are empty. Apply it to PoSAccountingData and drop the manual method; pos-accounting re-exports the trait so existing callers keep working. --- Cargo.lock | 16 ++++ .../chainstate_accounting_storage_tests.rs | 2 +- pos-accounting/Cargo.toml | 1 + pos-accounting/src/data.rs | 12 +-- pos-accounting/src/lib.rs | 3 + utils/is-empty-derive/Cargo.toml | 13 +++ utils/is-empty-derive/src/lib.rs | 77 ++++++++++++++++ utils/is-empty/Cargo.toml | 9 ++ utils/is-empty/src/lib.rs | 91 +++++++++++++++++++ 9 files changed, 213 insertions(+), 11 deletions(-) create mode 100644 utils/is-empty-derive/Cargo.toml create mode 100644 utils/is-empty-derive/src/lib.rs create mode 100644 utils/is-empty/Cargo.toml create mode 100644 utils/is-empty/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 5594180f0f..d36f97458e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4354,6 +4354,21 @@ dependencies = [ "serde", ] +[[package]] +name = "is-empty" +version = "1.4.0" +dependencies = [ + "is-empty-derive", +] + +[[package]] +name = "is-empty-derive" +version = "1.4.0" +dependencies = [ + "quote", + "syn 2.0.114", +] + [[package]] name = "is-terminal" version = "0.4.17" @@ -6642,6 +6657,7 @@ dependencies = [ "accounting", "common", "crypto", + "is-empty", "parity-scale-codec", "randomness", "rstest", diff --git a/chainstate/test-suite/src/tests/chainstate_accounting_storage_tests.rs b/chainstate/test-suite/src/tests/chainstate_accounting_storage_tests.rs index 5a53d9f976..434ea1fc32 100644 --- a/chainstate/test-suite/src/tests/chainstate_accounting_storage_tests.rs +++ b/chainstate/test-suite/src/tests/chainstate_accounting_storage_tests.rs @@ -32,7 +32,7 @@ use common::{ primitives::{Amount, Idable, per_thousand::PerThousand}, }; use crypto::vrf::{VRFKeyKind, VRFPrivateKey}; -use pos_accounting::PoolData; +use pos_accounting::{IsEmpty, PoolData}; use randomness::{CryptoRng, RngExt as _}; use rstest::rstest; use test_utils::random::{Seed, make_seedable_rng}; diff --git a/pos-accounting/Cargo.toml b/pos-accounting/Cargo.toml index 5fb643cb68..0bf647cb3d 100644 --- a/pos-accounting/Cargo.toml +++ b/pos-accounting/Cargo.toml @@ -9,6 +9,7 @@ rust-version.workspace = true accounting = { path = "../accounting" } common = { path = "../common" } crypto = { path = "../crypto" } +is-empty = { path = "../utils/is-empty" } randomness = { path = "../randomness" } serialization = { path = "../serialization" } typename = { path = "../utils/typename" } diff --git a/pos-accounting/src/data.rs b/pos-accounting/src/data.rs index 7371f692f4..0e8f87d5df 100644 --- a/pos-accounting/src/data.rs +++ b/pos-accounting/src/data.rs @@ -19,11 +19,12 @@ use common::{ chain::{DelegationId, PoolId}, primitives::Amount, }; +use is_empty::IsEmpty; use serialization::{Decode, Encode}; use crate::{DelegationData, PoolData}; -#[derive(Clone, Encode, Decode, Debug, PartialEq, Eq)] +#[derive(Clone, Encode, Decode, Debug, PartialEq, Eq, IsEmpty)] pub struct PoSAccountingData { /// A collection of all the pools and their data. pub pool_data: BTreeMap, @@ -50,13 +51,4 @@ impl PoSAccountingData { delegation_data: BTreeMap::new(), } } - - // TODO: avoid manual implementation (mintlayer/mintlayer-core#669) - pub fn is_empty(&self) -> bool { - self.pool_data.is_empty() - && self.pool_balances.is_empty() - && self.pool_delegation_shares.is_empty() - && self.delegation_balances.is_empty() - && self.delegation_data.is_empty() - } } diff --git a/pos-accounting/src/lib.rs b/pos-accounting/src/lib.rs index 8239d8e1da..52a849acc8 100644 --- a/pos-accounting/src/lib.rs +++ b/pos-accounting/src/lib.rs @@ -35,3 +35,6 @@ pub use crate::{ in_memory::InMemoryPoSAccounting, }, }; + +// Re-exported so that users of `PoSAccountingData` (which derives it) have the trait in scope. +pub use is_empty::IsEmpty; diff --git a/utils/is-empty-derive/Cargo.toml b/utils/is-empty-derive/Cargo.toml new file mode 100644 index 0000000000..b8b90dc35d --- /dev/null +++ b/utils/is-empty-derive/Cargo.toml @@ -0,0 +1,13 @@ +[package] +name = "is-empty-derive" +license.workspace = true +version.workspace = true +edition.workspace = true +rust-version.workspace = true + +[lib] +proc-macro = true + +[dependencies] +syn.workspace = true +quote.workspace = true diff --git a/utils/is-empty-derive/src/lib.rs b/utils/is-empty-derive/src/lib.rs new file mode 100644 index 0000000000..225fa8c5bd --- /dev/null +++ b/utils/is-empty-derive/src/lib.rs @@ -0,0 +1,77 @@ +// Copyright (c) 2026 RBB S.r.l +// opensource@mintlayer.org +// SPDX-License-Identifier: MIT +// Licensed under the MIT License; +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://github.com/mintlayer/mintlayer-core/blob/master/LICENSE +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use proc_macro::TokenStream; +use quote::quote; +use syn::{Data, DeriveInput, Fields, Index, parse_macro_input}; + +/// Derive the `IsEmpty` trait for a struct. +/// +/// The generated implementation considers the struct empty when all of its fields are empty. Each +/// field's own `is_empty` method is used, so every field type must have one (for example the +/// standard collections, `String`, or another type that implements `IsEmpty`). A struct with no +/// fields is always empty. +#[proc_macro_derive(IsEmpty)] +pub fn derive(input: TokenStream) -> TokenStream { + let DeriveInput { + ident, + generics, + data, + .. + } = parse_macro_input!(input); + + let fields = match data { + Data::Struct(data) => data.fields, + Data::Enum(_) | Data::Union(_) => { + return syn::Error::new_spanned(ident, "IsEmpty can only be derived for structs") + .to_compile_error() + .into(); + } + }; + + let checks: Vec<_> = match fields { + Fields::Named(fields) => fields + .named + .into_iter() + .map(|field| { + let name = field.ident.expect("a named field always has an identifier"); + quote!(self.#name.is_empty()) + }) + .collect(), + Fields::Unnamed(fields) => (0..fields.unnamed.len()) + .map(|i| { + let index = Index::from(i); + quote!(self.#index.is_empty()) + }) + .collect(), + Fields::Unit => Vec::new(), + }; + + let body = match checks.split_first() { + Some((first, rest)) => quote!(#first #(&& #rest)*), + None => quote!(true), + }; + + let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); + + quote! { + impl #impl_generics IsEmpty for #ident #ty_generics #where_clause { + fn is_empty(&self) -> bool { + #body + } + } + } + .into() +} diff --git a/utils/is-empty/Cargo.toml b/utils/is-empty/Cargo.toml new file mode 100644 index 0000000000..e7cac2074f --- /dev/null +++ b/utils/is-empty/Cargo.toml @@ -0,0 +1,9 @@ +[package] +name = "is-empty" +license.workspace = true +version.workspace = true +edition.workspace = true +rust-version.workspace = true + +[dependencies] +is-empty-derive = { path = "../is-empty-derive" } diff --git a/utils/is-empty/src/lib.rs b/utils/is-empty/src/lib.rs new file mode 100644 index 0000000000..2b1c2a64bd --- /dev/null +++ b/utils/is-empty/src/lib.rs @@ -0,0 +1,91 @@ +// Copyright (c) 2026 RBB S.r.l +// opensource@mintlayer.org +// SPDX-License-Identifier: MIT +// Licensed under the MIT License; +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://github.com/mintlayer/mintlayer-core/blob/master/LICENSE +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +pub use is_empty_derive::IsEmpty; + +/// The interface for checking whether a value is "empty". +/// +/// This is mainly meant to be derived for aggregate types (see the `IsEmpty` derive macro), where +/// a value is empty when all of its fields are empty. Deriving it instead of writing `is_empty` by +/// hand means a newly added field can't be silently left out of the check. +pub trait IsEmpty { + /// Returns `true` if the value is empty. + fn is_empty(&self) -> bool; +} + +#[cfg(test)] +mod tests { + use std::collections::BTreeMap; + + use super::*; + + #[derive(IsEmpty)] + struct Aggregate { + a: BTreeMap, + b: Vec, + c: String, + } + + #[test] + fn empty_when_all_fields_are_empty() { + let value = Aggregate { + a: BTreeMap::new(), + b: Vec::new(), + c: String::new(), + }; + assert!(value.is_empty()); + } + + #[test] + fn not_empty_when_any_field_is_not_empty() { + let with_map = Aggregate { + a: BTreeMap::from([(1, 1)]), + b: Vec::new(), + c: String::new(), + }; + assert!(!with_map.is_empty()); + + let with_vec = Aggregate { + a: BTreeMap::new(), + b: vec![1], + c: String::new(), + }; + assert!(!with_vec.is_empty()); + + let with_string = Aggregate { + a: BTreeMap::new(), + b: Vec::new(), + c: "x".to_owned(), + }; + assert!(!with_string.is_empty()); + } + + #[test] + fn tuple_struct_is_empty_uses_all_fields() { + #[derive(IsEmpty)] + struct Pair(Vec, String); + + assert!(Pair(Vec::new(), String::new()).is_empty()); + assert!(!Pair(vec![1], String::new()).is_empty()); + } + + #[test] + fn unit_struct_is_always_empty() { + #[derive(IsEmpty)] + struct Unit; + + assert!(Unit.is_empty()); + } +} From 5de506f31144dbacc281bdd748e79016afe4eb4f Mon Sep 17 00:00:00 2001 From: Oliver Anyanwu Date: Sat, 29 Aug 2026 13:39:22 +0100 Subject: [PATCH 2/2] Qualify the trait path in the IsEmpty derive The generated impl named the trait unqualified, so it resolved in the caller's scope. Deriving through a path, as in #[derive(is_empty::IsEmpty)], failed with E0405 unless the trait was also imported, and worse, if another trait called IsEmpty happened to be in scope the derive implemented that one instead and the type silently did not satisfy is_empty::IsEmpty. Emit ::is_empty::IsEmpty instead. The per field call stays unqualified on purpose, since the standard collections provide is_empty inherently. That path does not resolve inside the is-empty crate itself, so alias the crate to its own name for the tests, and cover both cases with a test that derives without the trait imported while a decoy IsEmpty is in scope. Also drop the IsEmpty re-export from pos-accounting, which existed only to give deriving users the trait in scope and is no longer needed. The one crate that calls the trait method now depends on is-empty directly. The doc comment claimed every field type must have an is_empty, which reads like a compile time guarantee. It is ordinary method lookup, so say what that means, including the fixed size array case that would silently make a struct permanently non-empty. --- Cargo.lock | 1 + chainstate/test-suite/Cargo.toml | 1 + .../chainstate_accounting_storage_tests.rs | 3 +- pos-accounting/src/lib.rs | 3 -- utils/is-empty-derive/src/lib.rs | 18 ++++++++--- utils/is-empty/src/lib.rs | 31 +++++++++++++++++++ 6 files changed, 48 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d36f97458e..7f18cfe052 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1514,6 +1514,7 @@ dependencies = [ "ctor", "expect-test", "hex", + "is-empty", "itertools 0.14.0", "logging", "orders-accounting", diff --git a/chainstate/test-suite/Cargo.toml b/chainstate/test-suite/Cargo.toml index 9f3660dd62..23b448fd2b 100644 --- a/chainstate/test-suite/Cargo.toml +++ b/chainstate/test-suite/Cargo.toml @@ -17,6 +17,7 @@ constraints-value-accumulator = { path = "../constraints-value-accumulator" } crypto = { path = "../../crypto" } logging = { path = "../../logging" } orders-accounting = { path = "../../orders-accounting" } +is-empty = { path = "../../utils/is-empty" } pos-accounting = { path = "../../pos-accounting" } randomness = { path = "../../randomness" } serialization = { path = "../../serialization" } diff --git a/chainstate/test-suite/src/tests/chainstate_accounting_storage_tests.rs b/chainstate/test-suite/src/tests/chainstate_accounting_storage_tests.rs index 434ea1fc32..fc97bfb21b 100644 --- a/chainstate/test-suite/src/tests/chainstate_accounting_storage_tests.rs +++ b/chainstate/test-suite/src/tests/chainstate_accounting_storage_tests.rs @@ -32,7 +32,8 @@ use common::{ primitives::{Amount, Idable, per_thousand::PerThousand}, }; use crypto::vrf::{VRFKeyKind, VRFPrivateKey}; -use pos_accounting::{IsEmpty, PoolData}; +use is_empty::IsEmpty; +use pos_accounting::PoolData; use randomness::{CryptoRng, RngExt as _}; use rstest::rstest; use test_utils::random::{Seed, make_seedable_rng}; diff --git a/pos-accounting/src/lib.rs b/pos-accounting/src/lib.rs index 52a849acc8..8239d8e1da 100644 --- a/pos-accounting/src/lib.rs +++ b/pos-accounting/src/lib.rs @@ -35,6 +35,3 @@ pub use crate::{ in_memory::InMemoryPoSAccounting, }, }; - -// Re-exported so that users of `PoSAccountingData` (which derives it) have the trait in scope. -pub use is_empty::IsEmpty; diff --git a/utils/is-empty-derive/src/lib.rs b/utils/is-empty-derive/src/lib.rs index 225fa8c5bd..c890b9cbf7 100644 --- a/utils/is-empty-derive/src/lib.rs +++ b/utils/is-empty-derive/src/lib.rs @@ -19,10 +19,18 @@ use syn::{Data, DeriveInput, Fields, Index, parse_macro_input}; /// Derive the `IsEmpty` trait for a struct. /// -/// The generated implementation considers the struct empty when all of its fields are empty. Each -/// field's own `is_empty` method is used, so every field type must have one (for example the -/// standard collections, `String`, or another type that implements `IsEmpty`). A struct with no -/// fields is always empty. +/// The generated implementation considers the struct empty when all of its fields are empty. A +/// struct with no fields is always empty. +/// +/// Each field's own `is_empty` method is used, and it is resolved by ordinary method lookup rather +/// than through this trait, so that the standard collections and `String` work through their +/// inherent methods. Two things follow from that: +/// +/// * A field whose `is_empty` means something unrelated is accepted silently. A fixed size array +/// is the easiest way to get this wrong, as `[T; N]` resolves to the slice method and returns +/// `false` for any `N > 0`, which would make the whole struct permanently non-empty. +/// * A field whose type gets its `is_empty` from this trait needs the trait in scope at the point +/// of the derive, since the per field call is not qualified. #[proc_macro_derive(IsEmpty)] pub fn derive(input: TokenStream) -> TokenStream { let DeriveInput { @@ -67,7 +75,7 @@ pub fn derive(input: TokenStream) -> TokenStream { let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); quote! { - impl #impl_generics IsEmpty for #ident #ty_generics #where_clause { + impl #impl_generics ::is_empty::IsEmpty for #ident #ty_generics #where_clause { fn is_empty(&self) -> bool { #body } diff --git a/utils/is-empty/src/lib.rs b/utils/is-empty/src/lib.rs index 2b1c2a64bd..f663cfe6cc 100644 --- a/utils/is-empty/src/lib.rs +++ b/utils/is-empty/src/lib.rs @@ -13,6 +13,10 @@ // See the License for the specific language governing permissions and // limitations under the License. +// The derive macro expands to `::is_empty::IsEmpty`, which would not resolve inside this crate +// itself. This alias makes that path work here too, so the trait can be derived in our own tests. +extern crate self as is_empty; + pub use is_empty_derive::IsEmpty; /// The interface for checking whether a value is "empty". @@ -88,4 +92,31 @@ mod tests { assert!(Unit.is_empty()); } + + // The generated impl names the trait by its full path, so deriving through a path qualified + // macro works without the trait being imported, and picks the right trait even if another one + // of the same name happens to be in scope. + mod trait_not_in_scope { + // Deliberately not importing the trait here. + + #[derive(super::super::IsEmpty)] + struct Aggregate { + a: Vec, + } + + // A decoy that would be picked up if the generated impl named the trait unqualified. + trait IsEmpty { + fn is_empty(&self) -> bool; + } + + fn assert_impls(value: &T) -> bool { + value.is_empty() + } + + #[test] + fn derives_the_trait_from_its_own_crate() { + assert!(assert_impls(&Aggregate { a: Vec::new() })); + assert!(!assert_impls(&Aggregate { a: vec![1] })); + } + } }