From f4aae3ac5d9ca30b92a0a5be3205a12e90c50e21 Mon Sep 17 00:00:00 2001 From: DCG-Claude Date: Wed, 16 Sep 2026 07:26:57 -0500 Subject: [PATCH 01/13] feat(dpp): add typed refund owners and record them on fee refunds Storage refunds were keyed by a bare 32-byte identifier that was always read as an identity. Contract credit buckets can also pay for storage, so the owner of stored bytes is now a typed RefundOwner (Identity or ContractBucket) with a deterministic carrier key: an identity keeps its id, a bucket derives a domain separated double SHA-256 of contract id and position. FeeRefunds keeps the historical carrier as its first field and records the typed owner of every carrier key in a second field; the untyped constructor records identities explicitly, the typed constructor reads the recorded map and rejects an unmapped key, and merging rejects one key that names two owners. Refs #4689 Co-Authored-By: Claude Fable 5.1 --- packages/rs-dpp/src/fee/fee_result/mod.rs | 28 +- packages/rs-dpp/src/fee/fee_result/refunds.rs | 444 +++++++++++++++++- packages/rs-dpp/src/fee/mod.rs | 1 + packages/rs-dpp/src/fee/refund_owner/mod.rs | 217 +++++++++ .../src/drive/identity/balance/update.rs | 34 +- .../v1/mod.rs | 7 +- .../src/drive/tokens/balance/update.rs | 25 +- packages/rs-drive/src/util/storage_flags.rs | 3 - 8 files changed, 736 insertions(+), 23 deletions(-) create mode 100644 packages/rs-dpp/src/fee/refund_owner/mod.rs delete mode 100644 packages/rs-drive/src/util/storage_flags.rs diff --git a/packages/rs-dpp/src/fee/fee_result/mod.rs b/packages/rs-dpp/src/fee/fee_result/mod.rs index 28b4a495f53..b753b1f7d2b 100644 --- a/packages/rs-dpp/src/fee/fee_result/mod.rs +++ b/packages/rs-dpp/src/fee/fee_result/mod.rs @@ -40,6 +40,7 @@ use crate::consensus::fee::fee_error::FeeError; use crate::fee::fee_result::refunds::FeeRefunds; use crate::fee::fee_result::BalanceChange::{AddToBalance, NoBalanceChange, RemoveFromBalance}; +use crate::fee::refund_owner::RefundOwner; use crate::fee::Credits; use crate::prelude::UserFeeIncrease; use crate::ProtocolError; @@ -137,6 +138,17 @@ impl BalanceChangeForIdentity { .calculate_all_refunds_except_identity(self.identity_id) } + /// Returns the refund amount of credits for every recorded owner other + /// than the paying identity, keyed by the typed owner. + /// + /// A carrier key without a recorded owner is an error: the refund cannot + /// be routed and must not be guessed at. + pub fn other_typed_refunds(&self) -> Result, ProtocolError> { + self.fee_result + .fee_refunds + .calculate_all_refunds_except_owner(&RefundOwner::Identity(self.identity_id)) + } + /// Convert into a fee result pub fn into_fee_result(self) -> FeeResult { self.fee_result @@ -308,7 +320,9 @@ mod tests { use super::*; use crate::consensus::fee::fee_error::FeeError; use crate::fee::epoch::CreditsPerEpoch; - use crate::fee::fee_result::refunds::{CreditsPerEpochByIdentifier, FeeRefunds}; + use crate::fee::fee_result::refunds::{ + CreditsPerEpochByIdentifier, FeeRefunds, RefundOwnersByIdentifier, + }; fn make_id(byte: u8) -> Identifier { Identifier::from([byte; 32]) @@ -320,7 +334,11 @@ mod tests { credits_per_epoch.insert(0, credits); let mut map = CreditsPerEpochByIdentifier::new(); map.insert(*identity_id.as_bytes(), credits_per_epoch); - FeeRefunds(map) + let owners = RefundOwnersByIdentifier::from([( + *identity_id.as_bytes(), + RefundOwner::Identity(identity_id), + )]); + FeeRefunds(map, owners) } // --- BalanceChangeForIdentity::change() --- @@ -366,7 +384,11 @@ mod tests { let mut map = CreditsPerEpochByIdentifier::new(); map.insert(*id.as_bytes(), credits_per_epoch_self); map.insert(*other_id.as_bytes(), credits_per_epoch_other); - let refunds = FeeRefunds(map); + let owners = RefundOwnersByIdentifier::from([ + (*id.as_bytes(), RefundOwner::Identity(id)), + (*other_id.as_bytes(), RefundOwner::Identity(other_id)), + ]); + let refunds = FeeRefunds(map, owners); let fee_result = FeeResult { storage_fee: 100, diff --git a/packages/rs-dpp/src/fee/fee_result/refunds.rs b/packages/rs-dpp/src/fee/fee_result/refunds.rs index 95142a4f82d..4fa61dcb7b6 100644 --- a/packages/rs-dpp/src/fee/fee_result/refunds.rs +++ b/packages/rs-dpp/src/fee/fee_result/refunds.rs @@ -2,12 +2,18 @@ //! //! Fee refunds are calculated based on removed bytes per epoch. //! +//! The carrier that GroveDB hands back keys removed bytes by a 32-byte +//! identifier. `FeeRefunds` keeps that carrier as its first field and records +//! the typed [`RefundOwner`] of every carrier key in its second field, so a +//! consumer that routes a refund reads the owner's kind from the record and +//! never infers it from the key. use crate::block::epoch::{Epoch, EpochIndex}; use crate::fee::default_costs::KnownCostItem::StorageDiskUsageCreditPerByte; use crate::fee::default_costs::{CachedEpochIndexFeeVersions, EpochCosts}; use crate::fee::epoch::distribution::calculate_storage_fee_refund_amount_and_leftovers; use crate::fee::epoch::{BytesPerEpoch, CreditsPerEpoch}; +use crate::fee::refund_owner::RefundOwner; use crate::fee::Credits; use crate::ProtocolError; use bincode::{Decode, DecodeUntrusted, Encode}; @@ -28,14 +34,29 @@ pub type CreditsPerEpochByIdentifier = BTreeMap<[u8; 32], CreditsPerEpoch>; /// Bytes per Epoch by Identifier pub type BytesPerEpochByIdentifier = BTreeMap<[u8; 32], BytesPerEpoch>; -/// Fee refunds to identities based on removed data from specific epochs +pub use crate::fee::refund_owner::RefundOwnersByIdentifier; + +/// Fee refunds to owners based on removed data from specific epochs. +/// +/// The first field is the per-owner, per-epoch credit carrier keyed by +/// [`RefundOwner::removal_key`]. The second field records the typed owner of +/// each carrier key. Every constructor that prices a storage removal fills +/// both; a key without a recorded owner cannot be routed and is treated as an +/// invariant failure by the typed accessors. #[derive( Debug, Clone, Eq, PartialEq, Default, Serialize, Deserialize, Encode, Decode, DecodeUntrusted, )] -pub struct FeeRefunds(pub CreditsPerEpochByIdentifier); +pub struct FeeRefunds( + pub CreditsPerEpochByIdentifier, + pub RefundOwnersByIdentifier, +); impl FeeRefunds { /// Create fee refunds from GroveDB's StorageRemovalPerEpochByIdentifier + /// + /// This is the untyped path: every carrier key is an identity id, as + /// written by the identity-owned storage flags, and is recorded as + /// [`RefundOwner::Identity`] explicitly. pub fn from_storage_removal( storage_removal: I, current_epoch_index: EpochIndex, @@ -47,7 +68,82 @@ impl FeeRefunds { C: IntoIterator, E: TryInto, { - let refunds_per_epoch_by_identifier = storage_removal + let refunds_per_epoch_by_identifier = Self::price_storage_removal( + storage_removal, + current_epoch_index, + epochs_per_era, + previous_fee_versions, + )?; + + let owners = refunds_per_epoch_by_identifier + .keys() + .map(|identifier| { + ( + *identifier, + RefundOwner::Identity(Identifier::from(*identifier)), + ) + }) + .collect(); + + Ok(Self(refunds_per_epoch_by_identifier, owners)) + } + + /// Create fee refunds from GroveDB's StorageRemovalPerEpochByIdentifier + /// with the owners recorded when the bytes were split. + /// + /// Every carrier key must have an entry in `owners`; a key without one is + /// a removal whose owner was never recorded, which is an invariant + /// failure rather than something to guess at. The system carrier key is + /// never an owner and must be removed by the caller before pricing. + pub fn from_typed_storage_removal( + storage_removal: I, + owners: &RefundOwnersByIdentifier, + current_epoch_index: EpochIndex, + epochs_per_era: u16, + previous_fee_versions: &CachedEpochIndexFeeVersions, + ) -> Result + where + I: IntoIterator, + C: IntoIterator, + E: TryInto, + { + let refunds_per_epoch_by_identifier = Self::price_storage_removal( + storage_removal, + current_epoch_index, + epochs_per_era, + previous_fee_versions, + )?; + + let recorded_owners = refunds_per_epoch_by_identifier + .keys() + .map(|identifier| { + owners + .get(identifier) + .map(|owner| (*identifier, *owner)) + .ok_or_else(|| { + ProtocolError::CorruptedCodeExecution(format!( + "storage removal carrier key {} has no recorded refund owner", + hex::encode(identifier) + )) + }) + }) + .collect::>()?; + + Ok(Self(refunds_per_epoch_by_identifier, recorded_owners)) + } + + fn price_storage_removal( + storage_removal: I, + current_epoch_index: EpochIndex, + epochs_per_era: u16, + previous_fee_versions: &CachedEpochIndexFeeVersions, + ) -> Result + where + I: IntoIterator, + C: IntoIterator, + E: TryInto, + { + storage_removal .into_iter() .map(|(identifier, bytes_per_epochs)| { bytes_per_epochs @@ -74,14 +170,17 @@ impl FeeRefunds { .collect::>() .map(|credits_per_epochs| (identifier, credits_per_epochs)) }) - .collect::>()?; - - Ok(Self(refunds_per_epoch_by_identifier)) + .collect::>() } /// Adds and self assigns result between two Fee Results + /// + /// Credits merge per carrier key and epoch. Recorded owners merge by key; + /// one carrier key naming two different owners is a collision that is + /// reported, never resolved by picking one. pub fn checked_add_assign(&mut self, rhs: Self) -> Result<(), ProtocolError> { - for (identifier, mut int_map_b) in rhs.0.into_iter() { + let Self(rhs_credits, rhs_owners) = rhs; + for (identifier, mut int_map_b) in rhs_credits.into_iter() { let to_insert_int_map = if let Some(sint_map_a) = self.0.remove(&identifier) { // other has an int_map with the same identifier let intersection = sint_map_a @@ -103,6 +202,22 @@ impl FeeRefunds { // reinsert the now combined IntMap self.0.insert(identifier, to_insert_int_map); } + for (identifier, owner) in rhs_owners { + match self.1.get(&identifier) { + Some(existing) if *existing != owner => { + return Err(ProtocolError::CorruptedCodeExecution(format!( + "storage removal carrier key {} is recorded for two different refund owners: {:?} and {:?}", + hex::encode(identifier), + existing, + owner + ))); + } + Some(_) => {} + None => { + self.1.insert(identifier, owner); + } + } + } Ok(()) } @@ -116,6 +231,32 @@ impl FeeRefunds { self.0.iter() } + /// The recorded owner of a carrier key + pub fn owner_of(&self, key: &[u8; 32]) -> Option { + self.1.get(key).copied() + } + + /// Iterates the refunds with their recorded owners. + /// + /// A carrier key without a recorded owner yields an error: a refund that + /// cannot name its owner cannot be routed and must halt rather than be + /// burned, minted or guessed. + pub fn iter_typed( + &self, + ) -> impl Iterator> + { + self.0.iter().map(|(identifier, credits_per_epoch)| { + self.owner_of(identifier) + .map(|owner| (owner, identifier, credits_per_epoch)) + .ok_or_else(|| { + ProtocolError::CorruptedCodeExecution(format!( + "storage refund carrier key {} has no recorded refund owner", + hex::encode(identifier) + )) + }) + }) + } + /// Sums the fee result among all identities pub fn sum_per_epoch(self) -> CreditsPerEpoch { let mut summed_credits = CreditsPerEpoch::default(); @@ -153,6 +294,34 @@ impl FeeRefunds { .collect() } + /// Calculates the refund amount of credits per recorded owner, excluding + /// the given owner. + /// + /// Owners are compared as typed values, so an identity and a contract + /// bucket never match each other. A carrier key without a recorded owner + /// is an error, as is a sum that overflows. + pub fn calculate_all_refunds_except_owner( + &self, + skip_owner: &RefundOwner, + ) -> Result, ProtocolError> { + let mut refunds_by_owner = BTreeMap::new(); + for entry in self.iter_typed() { + let (owner, _, credits_per_epoch) = entry?; + if owner == *skip_owner { + continue; + } + let credits = credits_per_epoch.values().try_fold(0u64, |sum, credits| { + sum.checked_add(*credits) + .ok_or(ProtocolError::Overflow("storage refund sum overflow")) + })?; + let total: &mut Credits = refunds_by_owner.entry(owner).or_insert(0); + *total = total + .checked_add(credits) + .ok_or(ProtocolError::Overflow("storage refund sum overflow"))?; + } + Ok(refunds_by_owner) + } + /// Calculates a refund amount of credits for specified identity id pub fn calculate_refunds_amount_for_identity( &self, @@ -178,16 +347,23 @@ impl IntoIterator for FeeRefunds { #[cfg(test)] mod tests { use super::*; + use nohash_hasher::IntMap; use once_cell::sync::Lazy; use platform_version::version::fee::FeeVersion; + use std::iter::FromIterator; static EPOCH_CHANGE_FEE_VERSION_TEST: Lazy = Lazy::new(|| BTreeMap::from([(0, FeeVersion::first())])); + fn bucket_owner(contract_byte: u8, position: u16) -> RefundOwner { + RefundOwner::ContractBucket { + contract_id: Identifier::from([contract_byte; 32]), + position, + } + } + mod from_storage_removal { use super::*; - use nohash_hasher::IntMap; - use std::iter::FromIterator; #[test] fn should_filter_out_refunds_under_the_limit() { @@ -210,5 +386,255 @@ mod tests { assert!(credits_per_epoch.get(&0).is_none()); assert!(credits_per_epoch.get(&1).is_some()); } + + #[test] + fn should_record_an_identity_owner_for_every_carrier_key() { + let first = [1u8; 32]; + let second = [2u8; 32]; + let storage_removal = BytesPerEpochByIdentifier::from_iter([ + (first, IntMap::from_iter([(0, 100)])), + (second, IntMap::from_iter([(1, 100)])), + ]); + + let fee_refunds = FeeRefunds::from_storage_removal( + storage_removal, + 3, + 20, + &EPOCH_CHANGE_FEE_VERSION_TEST, + ) + .expect("should create fee refunds"); + + assert_eq!( + fee_refunds.owner_of(&first), + Some(RefundOwner::Identity(Identifier::from(first))) + ); + assert_eq!( + fee_refunds.owner_of(&second), + Some(RefundOwner::Identity(Identifier::from(second))) + ); + assert_eq!(fee_refunds.1.len(), 2); + } + } + + mod from_typed_storage_removal { + use super::*; + + #[test] + fn should_record_the_owner_from_the_map_for_identity_and_bucket_keys() { + let identity = RefundOwner::Identity(Identifier::from([5u8; 32])); + let bucket = bucket_owner(6, 2); + let owners = RefundOwnersByIdentifier::from_iter([ + (identity.removal_key(), identity), + (bucket.removal_key(), bucket), + ]); + let storage_removal = BytesPerEpochByIdentifier::from_iter([ + (identity.removal_key(), IntMap::from_iter([(0, 100)])), + (bucket.removal_key(), IntMap::from_iter([(1, 200)])), + ]); + + let fee_refunds = FeeRefunds::from_typed_storage_removal( + storage_removal.clone(), + &owners, + 3, + 20, + &EPOCH_CHANGE_FEE_VERSION_TEST, + ) + .expect("should create fee refunds"); + + assert_eq!( + fee_refunds.owner_of(&identity.removal_key()), + Some(identity) + ); + assert_eq!(fee_refunds.owner_of(&bucket.removal_key()), Some(bucket)); + + // the credits are priced exactly as the untyped path prices them + let untyped = FeeRefunds::from_storage_removal( + storage_removal, + 3, + 20, + &EPOCH_CHANGE_FEE_VERSION_TEST, + ) + .expect("should create fee refunds"); + assert_eq!(fee_refunds.0, untyped.0); + } + + #[test] + fn should_reject_a_carrier_key_without_a_recorded_owner() { + let bucket = bucket_owner(6, 2); + let owners = RefundOwnersByIdentifier::new(); + let storage_removal = BytesPerEpochByIdentifier::from_iter([( + bucket.removal_key(), + IntMap::from_iter([(1, 200)]), + )]); + + let result = FeeRefunds::from_typed_storage_removal( + storage_removal, + &owners, + 3, + 20, + &EPOCH_CHANGE_FEE_VERSION_TEST, + ); + + assert!(matches!( + result, + Err(ProtocolError::CorruptedCodeExecution(_)) + )); + } + } + + mod checked_add_assign { + use super::*; + + fn refunds_for(owner: RefundOwner, epoch: u16, credits: Credits) -> FeeRefunds { + FeeRefunds( + CreditsPerEpochByIdentifier::from_iter([( + owner.removal_key(), + CreditsPerEpoch::from_iter([(epoch, credits)]), + )]), + RefundOwnersByIdentifier::from_iter([(owner.removal_key(), owner)]), + ) + } + + #[test] + fn should_merge_credits_and_owners_from_both_sides() { + let identity = RefundOwner::Identity(Identifier::from([1u8; 32])); + let bucket = bucket_owner(2, 0); + + let mut refunds = refunds_for(identity, 0, 10); + refunds + .checked_add_assign(refunds_for(identity, 1, 5)) + .expect("should merge the same owner"); + refunds + .checked_add_assign(refunds_for(bucket, 0, 7)) + .expect("should merge a second owner"); + + assert_eq!( + refunds.get(&identity.removal_key()), + Some(&CreditsPerEpoch::from_iter([(0, 10), (1, 5)])) + ); + assert_eq!( + refunds.get(&bucket.removal_key()), + Some(&CreditsPerEpoch::from_iter([(0, 7)])) + ); + assert_eq!(refunds.owner_of(&identity.removal_key()), Some(identity)); + assert_eq!(refunds.owner_of(&bucket.removal_key()), Some(bucket)); + } + + #[test] + fn should_reject_one_carrier_key_recorded_for_two_owners() { + let key = [9u8; 32]; + let identity = RefundOwner::Identity(Identifier::from(key)); + let bucket = bucket_owner(2, 0); + + let mut refunds = FeeRefunds( + CreditsPerEpochByIdentifier::from_iter([( + key, + CreditsPerEpoch::from_iter([(0, 10)]), + )]), + RefundOwnersByIdentifier::from_iter([(key, identity)]), + ); + let colliding = FeeRefunds( + CreditsPerEpochByIdentifier::from_iter([( + key, + CreditsPerEpoch::from_iter([(0, 1)]), + )]), + RefundOwnersByIdentifier::from_iter([(key, bucket)]), + ); + + let result = refunds.checked_add_assign(colliding); + + assert!(matches!( + result, + Err(ProtocolError::CorruptedCodeExecution(_)) + )); + } + } + + mod typed_accessors { + use super::*; + + #[test] + fn should_sum_refunds_per_owner_and_skip_the_given_owner() { + let payer = RefundOwner::Identity(Identifier::from([1u8; 32])); + let other = RefundOwner::Identity(Identifier::from([2u8; 32])); + let bucket = bucket_owner(3, 4); + + let refunds = FeeRefunds( + CreditsPerEpochByIdentifier::from_iter([ + (payer.removal_key(), CreditsPerEpoch::from_iter([(0, 100)])), + ( + other.removal_key(), + CreditsPerEpoch::from_iter([(0, 20), (1, 30)]), + ), + (bucket.removal_key(), CreditsPerEpoch::from_iter([(2, 7)])), + ]), + RefundOwnersByIdentifier::from_iter([ + (payer.removal_key(), payer), + (other.removal_key(), other), + (bucket.removal_key(), bucket), + ]), + ); + + let others = refunds + .calculate_all_refunds_except_owner(&payer) + .expect("should sum"); + + assert_eq!(others, BTreeMap::from_iter([(other, 50), (bucket, 7)])); + + // the identity view keeps its historical shape for identity keys + assert_eq!( + refunds.calculate_refunds_amount_for_identity(Identifier::from([2u8; 32])), + Some(50) + ); + let by_identity = + refunds.calculate_all_refunds_except_identity(Identifier::from([1u8; 32])); + assert_eq!(by_identity.get(&Identifier::from([2u8; 32])), Some(&50)); + } + + #[test] + fn should_fail_closed_on_a_carrier_key_without_a_recorded_owner() { + let refunds = FeeRefunds( + CreditsPerEpochByIdentifier::from_iter([( + [4u8; 32], + CreditsPerEpoch::from_iter([(0, 100)]), + )]), + RefundOwnersByIdentifier::new(), + ); + + let entries: Vec<_> = refunds.iter_typed().collect(); + assert_eq!(entries.len(), 1); + assert!(matches!( + entries[0], + Err(ProtocolError::CorruptedCodeExecution(_)) + )); + assert!(matches!( + refunds.calculate_all_refunds_except_owner(&bucket_owner(1, 1)), + Err(ProtocolError::CorruptedCodeExecution(_)) + )); + } + + #[test] + fn should_sum_per_epoch_across_owners_of_both_kinds() { + let identity = RefundOwner::Identity(Identifier::from([1u8; 32])); + let bucket = bucket_owner(3, 4); + let refunds = FeeRefunds( + CreditsPerEpochByIdentifier::from_iter([ + ( + identity.removal_key(), + CreditsPerEpoch::from_iter([(0, 100), (1, 1)]), + ), + (bucket.removal_key(), CreditsPerEpoch::from_iter([(0, 7)])), + ]), + RefundOwnersByIdentifier::from_iter([ + (identity.removal_key(), identity), + (bucket.removal_key(), bucket), + ]), + ); + + assert_eq!( + refunds.sum_per_epoch(), + CreditsPerEpoch::from_iter([(0, 107), (1, 1)]) + ); + } } } diff --git a/packages/rs-dpp/src/fee/mod.rs b/packages/rs-dpp/src/fee/mod.rs index f89ba6be42a..32d60d2a08c 100644 --- a/packages/rs-dpp/src/fee/mod.rs +++ b/packages/rs-dpp/src/fee/mod.rs @@ -2,5 +2,6 @@ pub mod default_costs; pub mod epoch; #[cfg(feature = "fee-distribution")] pub mod fee_result; +pub mod refund_owner; pub use crate::balances::credits::{Credits, SignedCredits}; diff --git a/packages/rs-dpp/src/fee/refund_owner/mod.rs b/packages/rs-dpp/src/fee/refund_owner/mod.rs new file mode 100644 index 00000000000..5a144f3138f --- /dev/null +++ b/packages/rs-dpp/src/fee/refund_owner/mod.rs @@ -0,0 +1,217 @@ +//! Refund owners +//! +//! Storage refunds are paid back to whoever paid for the bytes that were +//! freed. Historically that was always an identity and the owner was carried +//! as a bare 32-byte identifier. Contract credit buckets can also pay for +//! storage, so the owner of stored bytes is now a typed value whose kind is +//! recorded when the bytes are stored and never inferred from an identifier. +//! +//! The in-memory carrier that GroveDB hands back to Drive keys removed bytes +//! by a 32-byte identifier. [`RefundOwner::removal_key`] gives every owner +//! exactly one such carrier key: an identity's key is its identifier, a +//! contract bucket's key is a domain separated double SHA-256 of the contract +//! id and the bucket position. The recorded owner travels alongside the +//! carrier so that the kind is always read from the record, not from the key. + +use crate::util::hash::hash_double; +use bincode::{Decode, DecodeUntrusted, Encode}; +use platform_value::Identifier; +use serde::{Deserialize, Serialize}; +use std::collections::BTreeMap; + +/// Position of a credit bucket inside a contract's credit tree. +/// +/// Stored as a two-byte big-endian key under the contract's credit tree. +/// The contract credit tree lands with the contract credits work on this +/// release branch; this alias keeps the refund owner independent of its +/// arrival order and is the same width either way. +pub type ContractCreditBucketPosition = u16; + +/// Domain separator for the carrier key of a contract bucket refund owner. +/// +/// 35 ASCII bytes without a terminator. The trailing `/0` is the derivation +/// generation: a future change to the derivation is a new storage flag type +/// byte, so the derivation carries no separate version field. +/// +/// Provisional: proposed as the refund owner encoding allocation and pending +/// the owner's confirmation in the fees workstream register (issue 4689). +pub const REFUND_OWNER_CONTRACT_BUCKET_DOMAIN: &[u8; 35] = b"dash-platform/refund-owner/bucket/0"; + +/// The system carrier key. Bytes that no owner paid for are sectioned under +/// this key and are never refunded, so it is never a recorded owner. +pub const SYSTEM_REFUND_CARRIER_KEY: [u8; 32] = [0; 32]; + +/// The recorded refund owner of every carrier key of a storage removal +pub type RefundOwnersByIdentifier = BTreeMap<[u8; 32], RefundOwner>; + +/// The owner of stored bytes, recorded when the bytes are stored so that a +/// later removal knows where the refund goes. +/// +/// The kind is explicit. It is never derived from the shape or value of an +/// identifier. +#[derive( + Debug, + Clone, + Copy, + PartialEq, + Eq, + PartialOrd, + Ord, + Hash, + Serialize, + Deserialize, + Encode, + Decode, + DecodeUntrusted, +)] +pub enum RefundOwner { + /// An identity paid for the bytes. Refunds are credited to its balance + /// whether or not it may currently spend. + Identity(Identifier), + /// A contract credit bucket paid for the bytes. Refunds are credited to + /// the bucket on the contract credit path. + ContractBucket { + /// The contract whose credit tree holds the bucket. + contract_id: Identifier, + /// The bucket's position inside the contract credit tree. + position: ContractCreditBucketPosition, + }, +} + +impl RefundOwner { + /// The 32-byte key under which this owner's removed bytes are sectioned + /// in GroveDB's storage removal carrier. + /// + /// An identity's key is its identifier verbatim, so every historical + /// record keeps its key. A contract bucket's key is + /// `hash_double(domain || contract_id || position_be)` with the domain + /// from [`REFUND_OWNER_CONTRACT_BUCKET_DOMAIN`], 69 bytes of preimage + /// with no length prefixes. + pub fn removal_key(&self) -> [u8; 32] { + match self { + RefundOwner::Identity(identity_id) => identity_id.to_buffer(), + RefundOwner::ContractBucket { + contract_id, + position, + } => { + let mut preimage = + Vec::with_capacity(REFUND_OWNER_CONTRACT_BUCKET_DOMAIN.len() + 32 + 2); + preimage.extend_from_slice(REFUND_OWNER_CONTRACT_BUCKET_DOMAIN); + preimage.extend_from_slice(contract_id.as_bytes()); + preimage.extend_from_slice(&position.to_be_bytes()); + hash_double(preimage) + } + } + } + + /// The identity when this owner is an identity. + pub fn as_identity(&self) -> Option { + match self { + RefundOwner::Identity(identity_id) => Some(*identity_id), + RefundOwner::ContractBucket { .. } => None, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use bincode::config; + + #[test] + fn should_use_the_identity_id_verbatim_as_the_carrier_key() { + let identity_id = Identifier::from([7u8; 32]); + + assert_eq!(RefundOwner::Identity(identity_id).removal_key(), [7u8; 32]); + } + + #[test] + fn should_derive_the_pinned_carrier_key_for_a_contract_bucket() { + let owner = RefundOwner::ContractBucket { + contract_id: Identifier::from([0x11u8; 32]), + position: 7, + }; + + assert_eq!( + hex::encode(owner.removal_key()), + "e0cb6d5af10d1ece2cf9dccc39dac999ce9cd59e8c1314bc7017f19cfb9e5a1d" + ); + } + + #[test] + fn should_derive_the_bucket_key_from_the_documented_preimage() { + let contract_id = Identifier::from([0x11u8; 32]); + let owner = RefundOwner::ContractBucket { + contract_id, + position: 7, + }; + + let mut preimage = REFUND_OWNER_CONTRACT_BUCKET_DOMAIN.to_vec(); + preimage.extend_from_slice(&[0x11u8; 32]); + preimage.extend_from_slice(&7u16.to_be_bytes()); + assert_eq!(preimage.len(), 69); + + assert_eq!(owner.removal_key(), hash_double(preimage)); + } + + #[test] + fn should_give_distinct_carrier_keys_to_distinct_buckets() { + let contract_a = Identifier::from([1u8; 32]); + let contract_b = Identifier::from([2u8; 32]); + + let a0 = RefundOwner::ContractBucket { + contract_id: contract_a, + position: 0, + } + .removal_key(); + let a1 = RefundOwner::ContractBucket { + contract_id: contract_a, + position: 1, + } + .removal_key(); + let b0 = RefundOwner::ContractBucket { + contract_id: contract_b, + position: 0, + } + .removal_key(); + + assert_ne!(a0, a1); + assert_ne!(a0, b0); + assert_ne!(a1, b0); + assert_ne!(a0, SYSTEM_REFUND_CARRIER_KEY); + } + + #[test] + fn should_never_read_a_bucket_key_as_the_contract_id() { + let contract_id = Identifier::from([9u8; 32]); + let owner = RefundOwner::ContractBucket { + contract_id, + position: 0, + }; + + assert_ne!(owner.removal_key(), contract_id.to_buffer()); + assert_eq!(owner.as_identity(), None); + assert_eq!( + RefundOwner::Identity(contract_id).as_identity(), + Some(contract_id) + ); + } + + #[test] + fn should_round_trip_both_owner_kinds_through_bincode() { + let owners = [ + RefundOwner::Identity(Identifier::from([3u8; 32])), + RefundOwner::ContractBucket { + contract_id: Identifier::from([4u8; 32]), + position: u16::MAX, + }, + ]; + + for owner in owners { + let bytes = bincode::encode_to_vec(owner, config::standard()).expect("should encode"); + let (decoded, _): (RefundOwner, usize) = + bincode::decode_from_slice(&bytes, config::standard()).expect("should decode"); + assert_eq!(decoded, owner); + } + } +} diff --git a/packages/rs-drive/src/drive/identity/balance/update.rs b/packages/rs-drive/src/drive/identity/balance/update.rs index 959ff8240ca..b3226a0d2ca 100644 --- a/packages/rs-drive/src/drive/identity/balance/update.rs +++ b/packages/rs-drive/src/drive/identity/balance/update.rs @@ -523,6 +523,7 @@ mod tests { use dpp::fee::epoch::{CreditsPerEpoch, GENESIS_EPOCH_INDEX}; use dpp::fee::fee_result::refunds::{CreditsPerEpochByIdentifier, FeeRefunds}; use dpp::fee::fee_result::{BalanceChange, FeeResult}; + use dpp::fee::refund_owner::RefundOwner; use dpp::fee::{Credits, SignedCredits}; use dpp::version::PlatformVersion; use grovedb::batch::GroveOp; @@ -580,9 +581,19 @@ mod tests { (identity.id().to_buffer(), credits_per_epoch), (other_identity.id().to_buffer(), other_credits_per_epoch), ]); + let refund_owners = BTreeMap::from_iter([ + ( + identity.id().to_buffer(), + RefundOwner::Identity(identity.id()), + ), + ( + other_identity.id().to_buffer(), + RefundOwner::Identity(other_identity.id()), + ), + ]); let fee_result = FeeResult { - fee_refunds: FeeRefunds(refunds_per_epoch_by_identifier), + fee_refunds: FeeRefunds(refunds_per_epoch_by_identifier, refund_owners), ..Default::default() }; let fee_change = fee_result.clone().into_balance_change(identity.id()); @@ -658,11 +669,18 @@ mod tests { IntMap::from_iter([(GENESIS_EPOCH_INDEX, other_refund)]), ), ]); + let refund_owners = BTreeMap::from_iter([ + (payer.id().to_buffer(), RefundOwner::Identity(payer.id())), + ( + other_identity.id().to_buffer(), + RefundOwner::Identity(other_identity.id()), + ), + ]); let fee_result = FeeResult { storage_fee, processing_fee, - fee_refunds: FeeRefunds(refunds_per_epoch_by_identifier), + fee_refunds: FeeRefunds(refunds_per_epoch_by_identifier, refund_owners), ..Default::default() }; let fee_change = fee_result.clone().into_balance_change(payer.id()); @@ -749,9 +767,13 @@ mod tests { let refunds_per_epoch_by_identifier: CreditsPerEpochByIdentifier = BTreeMap::from_iter([(identity.id().to_buffer(), credits_per_epoch)]); + let refund_owners = BTreeMap::from_iter([( + identity.id().to_buffer(), + RefundOwner::Identity(identity.id()), + )]); let fee_result = FeeResult { - fee_refunds: FeeRefunds(refunds_per_epoch_by_identifier), + fee_refunds: FeeRefunds(refunds_per_epoch_by_identifier, refund_owners), ..Default::default() }; let fee_change = fee_result.clone().into_balance_change(identity.id()); @@ -826,9 +848,13 @@ mod tests { let refunds_per_epoch_by_identifier: CreditsPerEpochByIdentifier = BTreeMap::from_iter([(identity.id().to_buffer(), credits_per_epoch)]); + let refund_owners = BTreeMap::from_iter([( + identity.id().to_buffer(), + RefundOwner::Identity(identity.id()), + )]); let fee_result = FeeResult { - fee_refunds: FeeRefunds(refunds_per_epoch_by_identifier), + fee_refunds: FeeRefunds(refunds_per_epoch_by_identifier, refund_owners), ..Default::default() }; let fee_change = fee_result.clone().into_balance_change(identity.id()); diff --git a/packages/rs-drive/src/drive/identity/update/methods/apply_balance_change_from_fee_to_identity/v1/mod.rs b/packages/rs-drive/src/drive/identity/update/methods/apply_balance_change_from_fee_to_identity/v1/mod.rs index 6af90865d42..2a7a9b703aa 100644 --- a/packages/rs-drive/src/drive/identity/update/methods/apply_balance_change_from_fee_to_identity/v1/mod.rs +++ b/packages/rs-drive/src/drive/identity/update/methods/apply_balance_change_from_fee_to_identity/v1/mod.rs @@ -232,6 +232,7 @@ mod tests { use dpp::fee::epoch::GENESIS_EPOCH_INDEX; use dpp::fee::fee_result::refunds::{CreditsPerEpochByIdentifier, FeeRefunds}; use dpp::fee::fee_result::{BalanceChange, FeeResult}; + use dpp::fee::refund_owner::RefundOwner; use dpp::fee::Credits; use dpp::version::PlatformVersion; use nohash_hasher::IntMap; @@ -245,7 +246,11 @@ mod tests { IntMap::from_iter([(GENESIS_EPOCH_INDEX, credits)]), ) })); - FeeRefunds(refunds_per_epoch_by_identifier) + let refund_owners = BTreeMap::from_iter( + entries + .map(|(identity_id, _)| (identity_id, RefundOwner::Identity(identity_id.into()))), + ); + FeeRefunds(refunds_per_epoch_by_identifier, refund_owners) } #[test] diff --git a/packages/rs-drive/src/drive/tokens/balance/update.rs b/packages/rs-drive/src/drive/tokens/balance/update.rs index 18506d6f692..b969631bf1a 100644 --- a/packages/rs-drive/src/drive/tokens/balance/update.rs +++ b/packages/rs-drive/src/drive/tokens/balance/update.rs @@ -541,6 +541,7 @@ mod tests { use dpp::fee::epoch::{CreditsPerEpoch, GENESIS_EPOCH_INDEX}; use dpp::fee::fee_result::refunds::{CreditsPerEpochByIdentifier, FeeRefunds}; use dpp::fee::fee_result::FeeResult; + use dpp::fee::refund_owner::RefundOwner; use dpp::fee::{Credits, SignedCredits}; use dpp::version::PlatformVersion; use grovedb::batch::GroveOp; @@ -598,9 +599,19 @@ mod tests { (identity.id().to_buffer(), credits_per_epoch), (other_identity.id().to_buffer(), other_credits_per_epoch), ]); + let refund_owners = BTreeMap::from_iter([ + ( + identity.id().to_buffer(), + RefundOwner::Identity(identity.id()), + ), + ( + other_identity.id().to_buffer(), + RefundOwner::Identity(other_identity.id()), + ), + ]); let fee_result = FeeResult { - fee_refunds: FeeRefunds(refunds_per_epoch_by_identifier), + fee_refunds: FeeRefunds(refunds_per_epoch_by_identifier, refund_owners), ..Default::default() }; let fee_change = fee_result.clone().into_balance_change(identity.id()); @@ -694,9 +705,13 @@ mod tests { let refunds_per_epoch_by_identifier: CreditsPerEpochByIdentifier = BTreeMap::from_iter([(identity.id().to_buffer(), credits_per_epoch)]); + let refund_owners = BTreeMap::from_iter([( + identity.id().to_buffer(), + RefundOwner::Identity(identity.id()), + )]); let fee_result = FeeResult { - fee_refunds: FeeRefunds(refunds_per_epoch_by_identifier), + fee_refunds: FeeRefunds(refunds_per_epoch_by_identifier, refund_owners), ..Default::default() }; let fee_change = fee_result.clone().into_balance_change(identity.id()); @@ -771,9 +786,13 @@ mod tests { let refunds_per_epoch_by_identifier: CreditsPerEpochByIdentifier = BTreeMap::from_iter([(identity.id().to_buffer(), credits_per_epoch)]); + let refund_owners = BTreeMap::from_iter([( + identity.id().to_buffer(), + RefundOwner::Identity(identity.id()), + )]); let fee_result = FeeResult { - fee_refunds: FeeRefunds(refunds_per_epoch_by_identifier), + fee_refunds: FeeRefunds(refunds_per_epoch_by_identifier, refund_owners), ..Default::default() }; let fee_change = fee_result.clone().into_balance_change(identity.id()); diff --git a/packages/rs-drive/src/util/storage_flags.rs b/packages/rs-drive/src/util/storage_flags.rs deleted file mode 100644 index d933d14dac3..00000000000 --- a/packages/rs-drive/src/util/storage_flags.rs +++ /dev/null @@ -1,3 +0,0 @@ -pub use grovedb_epoch_based_storage_flags::{ - MergingOwnersStrategy, StorageFlags, SINGLE_EPOCH_FLAGS_SIZE, -}; From 93a8d648752df98ee059a5f50ea42996040d5b64 Mon Sep 17 00:00:00 2001 From: DCG-Claude Date: Wed, 16 Sep 2026 07:35:16 -0500 Subject: [PATCH 02/13] feat(drive): define storage flags with contract bucket owners in drive The storage flags module stops re-exporting the grovedb crate type and defines Drive's own StorageFlags. The four historical variants keep their names and shapes and are encoded and decoded through the pinned crate so their bytes cannot drift. Two variants are added for bytes a contract credit bucket paid for: type byte 4 (single epoch, 37 bytes) and type byte 5 (multi epoch). The parse helpers decode all six types so the shipped readers carry bucket flags through; the shipped-name batch closure entry points hand raw bytes to the crate and therefore reject the bucket types, so the shipped batch apply generation fails closed. The typed closure entry points section removed bytes under the owner's carrier key and record the owner, refusing one key that names two owners. Combine maps typed owners to their carrier keys for the crate's epoch arithmetic and maps the winner back through the inputs, so a replace with UseTheirs transfers ownership across kinds. The eleven direct crate imports switch to the Drive path. The GroveDB structure document gains an EpochContractBucket flags kind for type bytes 4 and 5, and grovedb-structure.json is regenerated for it. Refs #4689 Co-Authored-By: Claude Fable 5.1 Co-Authored-By: Claude Opus 5.5 --- packages/rs-drive/grovedb-structure.json | 5 + .../get_fetch/fetch_contract_ids/v0/mod.rs | 2 +- .../get_fetch/fetch_contracts/v0/mod.rs | 2 +- .../add_version_items_to_all_contracts.rs | 2 +- .../group/insert/add_group_action/v0/mod.rs | 2 +- packages/rs-drive/src/structure/flags.rs | 25 +- .../util/grove_operations/batch_move/mod.rs | 2 +- .../grove_operations/batch_move/v0/mod.rs | 2 +- .../batch_move_items_in_path_query/mod.rs | 2 +- .../batch_move_items_in_path_query/v0/mod.rs | 2 +- .../rs-drive/src/util/storage_flags/codec.rs | 288 +++++++ .../src/util/storage_flags/combine.rs | 142 ++++ .../rs-drive/src/util/storage_flags/mod.rs | 383 +++++++++ .../rs-drive/src/util/storage_flags/split.rs | 103 +++ .../rs-drive/src/util/storage_flags/tests.rs | 773 ++++++++++++++++++ .../rs-drive/src/util/storage_flags/update.rs | 88 ++ .../verify_contracts_by_range/v0/mod.rs | 2 +- .../verify_contracts_versions/v0/mod.rs | 2 +- 18 files changed, 1816 insertions(+), 11 deletions(-) create mode 100644 packages/rs-drive/src/util/storage_flags/codec.rs create mode 100644 packages/rs-drive/src/util/storage_flags/combine.rs create mode 100644 packages/rs-drive/src/util/storage_flags/mod.rs create mode 100644 packages/rs-drive/src/util/storage_flags/split.rs create mode 100644 packages/rs-drive/src/util/storage_flags/tests.rs create mode 100644 packages/rs-drive/src/util/storage_flags/update.rs diff --git a/packages/rs-drive/grovedb-structure.json b/packages/rs-drive/grovedb-structure.json index 4e4c8f0a377..edccd0c2005 100644 --- a/packages/rs-drive/grovedb-structure.json +++ b/packages/rs-drive/grovedb-structure.json @@ -175,6 +175,11 @@ "meaning": "Storage flags with an owner. They record the epoch in which the element's bytes were paid for, the bytes added in each later epoch if it grew, and the identity that paid. When the element is deleted or shrinks, the unused part of the storage fee is refunded to that identity, epoch by epoch.", "layout": "type byte 2, owner id 32 bytes, base epoch u16 BE; or type byte 3, owner id 32 bytes, base epoch u16 BE, then for each later epoch: epoch u16 BE and bytes added as a varint" }, + { + "name": "EpochContractBucket", + "meaning": "Storage flags owned by a contract credit bucket. They record the epoch in which the element's bytes were paid for, the bytes added in each later epoch if it grew, and the contract and bucket position that paid. When the element is deleted or shrinks, the unused part of the storage fee is refunded to that bucket, epoch by epoch.", + "layout": "type byte 4, contract id 32 bytes, bucket position u16 BE, base epoch u16 BE; or type byte 5, the same header, then for each later epoch: epoch u16 BE and bytes added as a varint" + }, { "name": "Other", "meaning": "Flags that are not Drive's storage flags.", diff --git a/packages/rs-drive/src/drive/contract/get_fetch/fetch_contract_ids/v0/mod.rs b/packages/rs-drive/src/drive/contract/get_fetch/fetch_contract_ids/v0/mod.rs index 3ac2776481f..afccd9e442e 100644 --- a/packages/rs-drive/src/drive/contract/get_fetch/fetch_contract_ids/v0/mod.rs +++ b/packages/rs-drive/src/drive/contract/get_fetch/fetch_contract_ids/v0/mod.rs @@ -43,12 +43,12 @@ impl Drive { #[cfg(test)] mod tests { + use crate::util::storage_flags::StorageFlags; use crate::util::test_helpers::setup::setup_drive_with_initial_state_structure; use dpp::block::block_info::BlockInfo; use dpp::data_contract::accessors::v0::DataContractV0Setters; use dpp::tests::json_document::json_document_to_contract; use dpp::version::PlatformVersion; - use grovedb_epoch_based_storage_flags::StorageFlags; fn setup_contracts(count: usize) -> (crate::drive::Drive, Vec<[u8; 32]>) { let drive = setup_drive_with_initial_state_structure(None); diff --git a/packages/rs-drive/src/drive/contract/get_fetch/fetch_contracts/v0/mod.rs b/packages/rs-drive/src/drive/contract/get_fetch/fetch_contracts/v0/mod.rs index 2e4251fd348..a57f717bdf2 100644 --- a/packages/rs-drive/src/drive/contract/get_fetch/fetch_contracts/v0/mod.rs +++ b/packages/rs-drive/src/drive/contract/get_fetch/fetch_contracts/v0/mod.rs @@ -101,13 +101,13 @@ impl Drive { #[cfg(test)] mod tests { + use crate::util::storage_flags::StorageFlags; use crate::util::test_helpers::setup::setup_drive_with_initial_state_structure; use dpp::block::block_info::BlockInfo; use dpp::data_contract::accessors::v0::{DataContractV0Getters, DataContractV0Setters}; use dpp::data_contract::config::v0::DataContractConfigSettersV0; use dpp::tests::json_document::json_document_to_contract; use dpp::version::PlatformVersion; - use grovedb_epoch_based_storage_flags::StorageFlags; fn setup_contracts(count: usize) -> (crate::drive::Drive, Vec<[u8; 32]>) { let drive = setup_drive_with_initial_state_structure(None); diff --git a/packages/rs-drive/src/drive/contract/migration/add_version_items_to_all_contracts.rs b/packages/rs-drive/src/drive/contract/migration/add_version_items_to_all_contracts.rs index ac7d164c0e0..080eb60dcf3 100644 --- a/packages/rs-drive/src/drive/contract/migration/add_version_items_to_all_contracts.rs +++ b/packages/rs-drive/src/drive/contract/migration/add_version_items_to_all_contracts.rs @@ -122,6 +122,7 @@ impl Drive { #[cfg(test)] mod tests { use crate::drive::Drive; + use crate::util::storage_flags::StorageFlags; use crate::util::test_helpers::setup::setup_drive_with_initial_state_structure; use dpp::block::block_info::BlockInfo; use dpp::data_contract::accessors::v0::{DataContractV0Getters, DataContractV0Setters}; @@ -129,7 +130,6 @@ mod tests { use dpp::data_contract::DataContract; use dpp::tests::json_document::json_document_to_contract; use dpp::version::PlatformVersion; - use grovedb_epoch_based_storage_flags::StorageFlags; const PLAIN_CONTRACT: &str = "tests/supporting_files/contract/family/family-contract.json"; const HISTORY_CONTRACT: &str = diff --git a/packages/rs-drive/src/drive/group/insert/add_group_action/v0/mod.rs b/packages/rs-drive/src/drive/group/insert/add_group_action/v0/mod.rs index 12ccb98337c..d0a8bf8ac70 100644 --- a/packages/rs-drive/src/drive/group/insert/add_group_action/v0/mod.rs +++ b/packages/rs-drive/src/drive/group/insert/add_group_action/v0/mod.rs @@ -13,6 +13,7 @@ use crate::util::grove_operations::{ }; use crate::util::object_size_info::PathKeyInfo::PathFixedSizeKeyRef; use crate::util::object_size_info::{DriveKeyInfo, PathKeyElementInfo}; +use crate::util::storage_flags::StorageFlags; use dpp::block::block_info::BlockInfo; use dpp::data_contract::group::GroupMemberPower; use dpp::data_contract::GroupContractPosition; @@ -26,7 +27,6 @@ use grovedb::batch::KeyInfoPath; use grovedb::element::SumValue; use grovedb::MaybeTree::NotTree; use grovedb::{Element, EstimatedLayerInformation, MaybeTree, TransactionArg, TreeType}; -use grovedb_epoch_based_storage_flags::StorageFlags; use std::collections::HashMap; impl Drive { diff --git a/packages/rs-drive/src/structure/flags.rs b/packages/rs-drive/src/structure/flags.rs index 94354cad628..95406025cd8 100644 --- a/packages/rs-drive/src/structure/flags.rs +++ b/packages/rs-drive/src/structure/flags.rs @@ -18,16 +18,23 @@ pub enum FlagsKind { /// Storage flags with an owner: the same, plus the identity a refund goes /// to. EpochOwned, + /// Storage flags owned by a contract credit bucket: the same epochs, plus + /// the contract and bucket position a refund goes to. + /// + /// Provisional: the type bytes 4 and 5 are proposed in the fees workstream + /// register (issue 4689) and pending the owner's confirmation. + EpochContractBucket, /// Flags that are not storage flags Other, } impl FlagsKind { /// Every kind, in declaration order - pub const ALL: [FlagsKind; 4] = [ + pub const ALL: [FlagsKind; 5] = [ FlagsKind::None, FlagsKind::Epoch, FlagsKind::EpochOwned, + FlagsKind::EpochContractBucket, FlagsKind::Other, ]; @@ -45,6 +52,10 @@ impl FlagsKind { Ok(Some(StorageFlags::SingleEpochOwned(..) | StorageFlags::MultiEpochOwned(..))) => { FlagsKind::EpochOwned } + Ok(Some( + StorageFlags::SingleEpochContractBucket(..) + | StorageFlags::MultiEpochContractBucket(..), + )) => FlagsKind::EpochContractBucket, Err(_) => FlagsKind::Other, } } @@ -72,6 +83,13 @@ impl FlagsKind { identity that paid. When the element is deleted or shrinks, the unused part \ of the storage fee is refunded to that identity, epoch by epoch." } + FlagsKind::EpochContractBucket => { + "Storage flags owned by a contract credit bucket. They record the epoch in \ + which the element's bytes were paid for, the bytes added in each later epoch \ + if it grew, and the contract and bucket position that paid. When the element \ + is deleted or shrinks, the unused part of the storage fee is refunded to that \ + bucket, epoch by epoch." + } FlagsKind::Other => "Flags that are not Drive's storage flags.", } } @@ -89,6 +107,11 @@ impl FlagsKind { 32 bytes, base epoch u16 BE, then for each later epoch: epoch u16 BE and bytes \ added as a varint" } + FlagsKind::EpochContractBucket => { + "type byte 4, contract id 32 bytes, bucket position u16 BE, base epoch u16 BE; \ + or type byte 5, the same header, then for each later epoch: epoch u16 BE and \ + bytes added as a varint" + } FlagsKind::Other => "unknown", } } diff --git a/packages/rs-drive/src/util/grove_operations/batch_move/mod.rs b/packages/rs-drive/src/util/grove_operations/batch_move/mod.rs index eb8b64d8214..e6d234cf090 100644 --- a/packages/rs-drive/src/util/grove_operations/batch_move/mod.rs +++ b/packages/rs-drive/src/util/grove_operations/batch_move/mod.rs @@ -5,9 +5,9 @@ use crate::error::drive::DriveError; use crate::error::Error; use crate::fees::op::LowLevelDriveOperation; use crate::util::grove_operations::BatchMoveApplyType; +use crate::util::storage_flags::StorageFlags; use dpp::version::drive_versions::DriveVersion; use grovedb::TransactionArg; -use grovedb_epoch_based_storage_flags::StorageFlags; use grovedb_path::SubtreePath; impl Drive { diff --git a/packages/rs-drive/src/util/grove_operations/batch_move/v0/mod.rs b/packages/rs-drive/src/util/grove_operations/batch_move/v0/mod.rs index ff535afa818..088630edba8 100644 --- a/packages/rs-drive/src/util/grove_operations/batch_move/v0/mod.rs +++ b/packages/rs-drive/src/util/grove_operations/batch_move/v0/mod.rs @@ -5,12 +5,12 @@ use crate::fees::op::LowLevelDriveOperation; use crate::fees::op::LowLevelDriveOperation::GroveOperation; use crate::util::grove_operations::pending_grove_operations::pending_grove_operations; use crate::util::grove_operations::{push_drive_operation_result, BatchMoveApplyType, QueryType}; +use crate::util::storage_flags::StorageFlags; use grovedb::batch::key_info::KeyInfo; use grovedb::batch::{KeyInfoPath, QualifiedGroveDbOp}; use grovedb::operations::delete::DeleteOptions; use grovedb::BackwardsReferences; use grovedb::{Element, GroveDb, TransactionArg}; -use grovedb_epoch_based_storage_flags::StorageFlags; use grovedb_path::SubtreePath; use grovedb_storage::rocksdb_storage::RocksDbStorage; use platform_version::version::drive_versions::DriveVersion; diff --git a/packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/mod.rs b/packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/mod.rs index 7c31d0b7a66..f65f9ad6a8f 100644 --- a/packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/mod.rs +++ b/packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/mod.rs @@ -8,8 +8,8 @@ use crate::util::grove_operations::BatchMoveApplyType; use dpp::version::drive_versions::DriveVersion; +use crate::util::storage_flags::StorageFlags; use grovedb::{PathQuery, TransactionArg}; -use grovedb_epoch_based_storage_flags::StorageFlags; impl Drive { /// Pushes multiple "delete element" and "insert element operations for items in a given path based on a `PathQuery` to `drive_operations`. diff --git a/packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/v0/mod.rs b/packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/v0/mod.rs index 590fce6a40d..b0caea9c4af 100644 --- a/packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/v0/mod.rs +++ b/packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/v0/mod.rs @@ -5,13 +5,13 @@ use crate::fees::op::LowLevelDriveOperation; use crate::fees::op::LowLevelDriveOperation::GroveOperation; use crate::util::grove_operations::pending_grove_operations::pending_grove_operations; use crate::util::grove_operations::{push_drive_operation_result, BatchMoveApplyType}; +use crate::util::storage_flags::StorageFlags; use grovedb::batch::key_info::KeyInfo; use grovedb::batch::{KeyInfoPath, QualifiedGroveDbOp}; use grovedb::operations::delete::DeleteOptions; use grovedb::query_result_type::QueryResultType; use grovedb::BackwardsReferences; use grovedb::{GroveDb, PathQuery, TransactionArg}; -use grovedb_epoch_based_storage_flags::StorageFlags; use grovedb_storage::rocksdb_storage::RocksDbStorage; use platform_version::version::drive_versions::DriveVersion; diff --git a/packages/rs-drive/src/util/storage_flags/codec.rs b/packages/rs-drive/src/util/storage_flags/codec.rs new file mode 100644 index 00000000000..c76aa2ba7d0 --- /dev/null +++ b/packages/rs-drive/src/util/storage_flags/codec.rs @@ -0,0 +1,288 @@ +//! Serialization of storage flags. +//! +//! Types 0 to 3 are produced and parsed by the pinned +//! `grovedb_epoch_based_storage_flags` crate so that their bytes cannot +//! drift from what is on chain. Types 4 and 5 are the contract bucket +//! variants defined here. + +use super::{ + BaseEpoch, BytesAddedInEpoch, ContractId, CrateStorageFlags, EpochIndex, StorageFlags, + CONTRACT_BUCKET_OWNER_SIZE, SINGLE_EPOCH_FLAGS_SIZE, +}; +use dpp::fee::refund_owner::ContractCreditBucketPosition; +use grovedb::ElementFlags; +use grovedb_epoch_based_storage_flags::error::StorageFlagsError; +use integer_encoding::VarInt; +use std::borrow::Cow; +use std::collections::BTreeMap; + +/// Type byte of the single epoch contract bucket variant +const SINGLE_EPOCH_CONTRACT_BUCKET_TYPE: u8 = 4; + +/// Type byte of the multi epoch contract bucket variant +const MULTI_EPOCH_CONTRACT_BUCKET_TYPE: u8 = 5; + +/// Size of the fixed header of both contract bucket variants: type byte, +/// contract id, position and base epoch +const CONTRACT_BUCKET_HEADER_SIZE: usize = + (SINGLE_EPOCH_FLAGS_SIZE + CONTRACT_BUCKET_OWNER_SIZE) as usize; + +/// Smallest possible epoch map entry: two epoch index bytes and one varint byte +const MIN_EPOCH_MAP_ENTRY_SIZE: usize = 3; + +impl StorageFlags { + /// Serialize storage flags + pub fn serialize(&self) -> Vec { + match self { + StorageFlags::SingleEpochContractBucket(base_epoch, contract_id, position) => { + let mut buffer = Vec::with_capacity(CONTRACT_BUCKET_HEADER_SIZE); + Self::append_contract_bucket_header( + &mut buffer, + SINGLE_EPOCH_CONTRACT_BUCKET_TYPE, + contract_id, + *position, + *base_epoch, + ); + buffer + } + StorageFlags::MultiEpochContractBucket(base_epoch, epochs, contract_id, position) => { + let mut buffer = Vec::with_capacity( + CONTRACT_BUCKET_HEADER_SIZE + epochs.len() * MIN_EPOCH_MAP_ENTRY_SIZE, + ); + Self::append_contract_bucket_header( + &mut buffer, + MULTI_EPOCH_CONTRACT_BUCKET_TYPE, + contract_id, + *position, + *base_epoch, + ); + Self::append_epoch_map(&mut buffer, epochs); + buffer + } + // the four historical variants are exactly the crate's + _ => self.to_crate_flags_keyed_by_removal_key().serialize(), + } + } + + /// Serialized size of storage flags, equal to `serialize().len()` + pub fn serialized_size(&self) -> u32 { + match self { + StorageFlags::SingleEpochContractBucket(..) => CONTRACT_BUCKET_HEADER_SIZE as u32, + StorageFlags::MultiEpochContractBucket(_, epochs, ..) => { + CONTRACT_BUCKET_HEADER_SIZE as u32 + Self::epoch_map_size(epochs) + } + _ => self.to_crate_flags_keyed_by_removal_key().serialized_size(), + } + } + + fn append_contract_bucket_header( + buffer: &mut Vec, + type_byte: u8, + contract_id: &ContractId, + position: ContractCreditBucketPosition, + base_epoch: BaseEpoch, + ) { + buffer.push(type_byte); + buffer.extend_from_slice(contract_id); + buffer.extend_from_slice(&position.to_be_bytes()); + buffer.extend_from_slice(&base_epoch.to_be_bytes()); + } + + fn append_epoch_map(buffer: &mut Vec, epochs: &BTreeMap) { + epochs.iter().for_each(|(epoch_index, bytes_added)| { + buffer.extend_from_slice(&epoch_index.to_be_bytes()); + buffer.extend(bytes_added.encode_var_vec()); + }) + } + + fn epoch_map_size(epochs: &BTreeMap) -> u32 { + epochs + .values() + .map(|bytes_added| 2 + bytes_added.encode_var_vec().len() as u32) + .sum() + } + + fn deserialize_contract_bucket_header( + data: &[u8], + ) -> Result<(BaseEpoch, ContractId, ContractCreditBucketPosition), StorageFlagsError> { + let contract_id: ContractId = data + .get(1..33) + .and_then(|bytes| bytes.try_into().ok()) + .ok_or_else(|| { + StorageFlagsError::StorageFlagsWrongSize( + "contract bucket flags must have 32 bytes of contract id".to_string(), + ) + })?; + let position = data + .get(33..35) + .and_then(|bytes| bytes.try_into().ok()) + .map(u16::from_be_bytes) + .ok_or_else(|| { + StorageFlagsError::StorageFlagsWrongSize( + "contract bucket flags must have 2 bytes of bucket position".to_string(), + ) + })?; + let base_epoch = data + .get(35..37) + .and_then(|bytes| bytes.try_into().ok()) + .map(u16::from_be_bytes) + .ok_or_else(|| { + StorageFlagsError::StorageFlagsWrongSize( + "contract bucket flags must have 2 bytes of base epoch".to_string(), + ) + })?; + Ok((base_epoch, contract_id, position)) + } + + /// Deserialize single epoch contract bucket storage flags from bytes + fn deserialize_single_epoch_contract_bucket(data: &[u8]) -> Result { + if data.len() != CONTRACT_BUCKET_HEADER_SIZE { + return Err(StorageFlagsError::StorageFlagsWrongSize( + "single epoch contract bucket must be 37 bytes total".to_string(), + )); + } + let (base_epoch, contract_id, position) = Self::deserialize_contract_bucket_header(data)?; + Ok(StorageFlags::SingleEpochContractBucket( + base_epoch, + contract_id, + position, + )) + } + + /// Deserialize multi epoch contract bucket storage flags from bytes. + /// + /// Unlike the crate's multi epoch decoders this one is strict: the epoch + /// map must hold at least one entry and the bytes must end exactly at + /// the end of the last entry. + fn deserialize_multi_epoch_contract_bucket(data: &[u8]) -> Result { + let len = data.len(); + if len < CONTRACT_BUCKET_HEADER_SIZE + MIN_EPOCH_MAP_ENTRY_SIZE { + return Err(StorageFlagsError::StorageFlagsWrongSize( + "multi epoch contract bucket must be at least 40 bytes total".to_string(), + )); + } + let (base_epoch, contract_id, position) = Self::deserialize_contract_bucket_header(data)?; + let mut offset = CONTRACT_BUCKET_HEADER_SIZE; + let mut bytes_per_epoch: BTreeMap = BTreeMap::default(); + while offset < len { + let epoch_index = data + .get(offset..offset + 2) + .and_then(|bytes| bytes.try_into().ok()) + .map(u16::from_be_bytes) + .ok_or_else(|| { + StorageFlagsError::StorageFlagsWrongSize( + "multi epoch contract bucket must have enough bytes for epoch indexes" + .to_string(), + ) + })?; + offset += 2; + let (bytes_at_epoch, bytes_used) = data + .get(offset..) + .and_then(u32::decode_var) + .ok_or_else(|| { + StorageFlagsError::StorageFlagsWrongSize( + "multi epoch contract bucket must have enough bytes for the amount of bytes used" + .to_string(), + ) + })?; + offset += bytes_used; + bytes_per_epoch.insert(epoch_index, bytes_at_epoch); + } + if bytes_per_epoch.is_empty() { + return Err(StorageFlagsError::StorageFlagsWrongSize( + "multi epoch contract bucket must carry at least one epoch entry".to_string(), + )); + } + Ok(StorageFlags::MultiEpochContractBucket( + base_epoch, + bytes_per_epoch, + contract_id, + position, + )) + } + + /// Deserialize storage flags from bytes. + /// + /// Decodes all six variants: the four historical ones through the crate, + /// the two contract bucket ones here. An unknown type byte is an error. + pub fn deserialize(data: &[u8]) -> Result, StorageFlagsError> { + match data.first() { + None => Ok(None), + Some(&SINGLE_EPOCH_CONTRACT_BUCKET_TYPE) => { + Ok(Some(Self::deserialize_single_epoch_contract_bucket(data)?)) + } + Some(&MULTI_EPOCH_CONTRACT_BUCKET_TYPE) => { + Ok(Some(Self::deserialize_multi_epoch_contract_bucket(data)?)) + } + Some(_) => Ok(CrateStorageFlags::deserialize(data)?.map(Self::from)), + } + } + + /// Creates storage flags from a slice. + pub fn from_slice(data: &[u8]) -> Result, StorageFlagsError> { + Self::deserialize(data) + } + + /// Creates storage flags from element flags. + pub fn from_element_flags_ref(data: &ElementFlags) -> Result, StorageFlagsError> { + Self::from_slice(data.as_slice()) + } + + /// Create Storage flags from optional element flags ref + pub fn map_some_element_flags_ref( + data: &Option, + ) -> Result, StorageFlagsError> { + match data { + None => Ok(None), + Some(data) => Self::from_slice(data.as_slice()), + } + } + + /// Create Storage flags from optional element flags ref + pub fn map_cow_some_element_flags_ref( + data: &Option, + ) -> Result>, StorageFlagsError> { + match data { + None => Ok(None), + Some(data) => Self::from_slice(data.as_slice()).map(|option| option.map(Cow::Owned)), + } + } + + /// Map to owned optional element flags + pub fn map_owned_to_element_flags(maybe_storage_flags: Option) -> ElementFlags { + maybe_storage_flags + .map(|storage_flags| storage_flags.serialize()) + .unwrap_or_default() + } + + /// Map to optional element flags + pub fn map_to_some_element_flags(maybe_storage_flags: Option<&Self>) -> Option { + maybe_storage_flags.map(|storage_flags| storage_flags.serialize()) + } + + /// Map to optional element flags + pub fn map_cow_to_some_element_flags( + maybe_storage_flags: Option>, + ) -> Option { + maybe_storage_flags.map(|storage_flags| storage_flags.serialize()) + } + + /// Map to optional element flags + pub fn map_borrowed_cow_to_some_element_flags( + maybe_storage_flags: &Option>, + ) -> Option { + maybe_storage_flags + .as_ref() + .map(|storage_flags| storage_flags.serialize()) + } + + /// Creates optional element flags + pub fn to_some_element_flags(&self) -> Option { + Some(self.serialize()) + } + + /// Creates element flags. + pub fn to_element_flags(&self) -> ElementFlags { + self.serialize() + } +} diff --git a/packages/rs-drive/src/util/storage_flags/combine.rs b/packages/rs-drive/src/util/storage_flags/combine.rs new file mode 100644 index 00000000000..7f331befef0 --- /dev/null +++ b/packages/rs-drive/src/util/storage_flags/combine.rs @@ -0,0 +1,142 @@ +//! Combining storage flags when an element is replaced. +//! +//! The epoch arithmetic is the crate's. The typed owner of each side is +//! replaced by its carrier key for the duration of the crate call and mapped +//! back through the inputs afterwards, so the crate's owner comparison +//! (equality of 32-byte ids) keeps two different owners apart and the kind +//! of the winning owner is read from the input that supplied it, never from +//! the key. + +use super::{CrateStorageFlags, MergingOwnersStrategy, StorageFlags}; +use dpp::fee::refund_owner::RefundOwner; +use grovedb_costs::storage_cost::removal::StorageRemovedBytes; +use grovedb_epoch_based_storage_flags::error::StorageFlagsError; + +impl StorageFlags { + /// Optional combine added bytes + pub fn optional_combine_added_bytes( + ours: Option, + theirs: Self, + added_bytes: u32, + merging_owners_strategy: MergingOwnersStrategy, + ) -> Result { + match ours { + None => Ok(theirs), + Some(ours) => ours.combine_added_bytes(theirs, added_bytes, merging_owners_strategy), + } + } + + /// Optional combine removed bytes + pub fn optional_combine_removed_bytes( + ours: Option, + theirs: Self, + removed_bytes: &StorageRemovedBytes, + merging_owners_strategy: MergingOwnersStrategy, + ) -> Result { + match ours { + None => Ok(theirs), + Some(ours) => { + ours.combine_removed_bytes(theirs, removed_bytes, merging_owners_strategy) + } + } + } + + /// Combine added bytes + pub fn combine_added_bytes( + self, + rhs: Self, + added_bytes: u32, + merging_owners_strategy: MergingOwnersStrategy, + ) -> Result { + let ours_owner = self.refund_owner(); + let theirs_owner = rhs.refund_owner(); + Self::reject_colliding_owners(ours_owner, theirs_owner)?; + let combined = self + .to_crate_flags_keyed_by_removal_key() + .combine_added_bytes( + rhs.to_crate_flags_keyed_by_removal_key(), + added_bytes, + merging_owners_strategy, + )?; + Self::from_crate_combined(combined, ours_owner, theirs_owner) + } + + /// Combine removed bytes + pub fn combine_removed_bytes( + self, + rhs: Self, + removed_bytes: &StorageRemovedBytes, + merging_owners_strategy: MergingOwnersStrategy, + ) -> Result { + let ours_owner = self.refund_owner(); + let theirs_owner = rhs.refund_owner(); + Self::reject_colliding_owners(ours_owner, theirs_owner)?; + let combined = self + .to_crate_flags_keyed_by_removal_key() + .combine_removed_bytes( + rhs.to_crate_flags_keyed_by_removal_key(), + removed_bytes, + merging_owners_strategy, + )?; + Self::from_crate_combined(combined, ours_owner, theirs_owner) + } + + /// Two different typed owners with one carrier key can never be told + /// apart by the crate, so the merge is refused before it starts. + fn reject_colliding_owners( + ours: Option, + theirs: Option, + ) -> Result<(), StorageFlagsError> { + if let (Some(ours), Some(theirs)) = (ours, theirs) { + if ours != theirs && ours.removal_key() == theirs.removal_key() { + return Err(StorageFlagsError::MergingStorageFlagsFromDifferentOwners( + "two different refund owners share one storage removal carrier key".to_string(), + )); + } + } + Ok(()) + } + + /// Maps the crate's combined flags back to the typed owner that supplied + /// the winning carrier key. + fn from_crate_combined( + combined: CrateStorageFlags, + ours: Option, + theirs: Option, + ) -> Result { + let Some(key) = combined.owner_id().copied() else { + return Ok(Self::from(combined)); + }; + let owner = [ours, theirs] + .into_iter() + .flatten() + .find(|owner| owner.removal_key() == key) + .ok_or_else(|| { + StorageFlagsError::MergingStorageFlagsFromDifferentOwners( + "combined storage flags name an owner that neither side supplied".to_string(), + ) + })?; + match owner { + RefundOwner::Identity(_) => Ok(Self::from(combined)), + RefundOwner::ContractBucket { + contract_id, + position, + } => { + let base_epoch = *combined.base_epoch(); + Ok(match combined.epoch_index_map() { + None => StorageFlags::SingleEpochContractBucket( + base_epoch, + contract_id.to_buffer(), + position, + ), + Some(epochs) => StorageFlags::MultiEpochContractBucket( + base_epoch, + epochs.clone(), + contract_id.to_buffer(), + position, + ), + }) + } + } + } +} diff --git a/packages/rs-drive/src/util/storage_flags/mod.rs b/packages/rs-drive/src/util/storage_flags/mod.rs new file mode 100644 index 00000000000..e354306ce1f --- /dev/null +++ b/packages/rs-drive/src/util/storage_flags/mod.rs @@ -0,0 +1,383 @@ +//! Storage flags +//! +//! Element flags are the opaque bytes GroveDB stores next to every element. +//! Drive uses them to remember which epoch paid for the bytes and who paid, +//! so that a later removal can refund the right owner at the right rate. +//! +//! GroveDB never interprets these bytes; only the closures Drive passes to +//! the batch apply and the readers in this crate do. That is why the codec +//! lives here. The four historical variants keep the exact encoding of the +//! `grovedb_epoch_based_storage_flags` crate: their bytes are produced and +//! parsed by that crate, so they cannot drift. Two variants are added for +//! bytes paid for by a contract credit bucket. The type byte declares the +//! owner's kind; it is never inferred from the shape of an identifier. +//! +//! Two groups of entry points differ in what they accept. The parse helpers +//! (`deserialize`, `from_element_flags_ref`, the `map_*` helpers) decode all +//! six variants so that readers carry bucket-owned flags through unchanged. +//! The batch closure entry points that keep their historical names +//! (`update_element_flags`, `split_removal_bytes`) hand the raw bytes to the +//! crate and therefore reject the two bucket variants: the batch apply +//! generation that predates typed owners fails closed on flags it was never +//! written to price. The typed closure entry points +//! (`update_element_flags_typed`, `split_removal_bytes_typed`) accept all +//! six variants and record the owner of every sectioned removal. + +mod codec; +mod combine; +mod split; +mod update; + +use dpp::fee::refund_owner::{ContractCreditBucketPosition, RefundOwner}; +use dpp::identifier::Identifier; +use grovedb_epoch_based_storage_flags::StorageFlags as CrateStorageFlags; +pub use grovedb_epoch_based_storage_flags::{ + MergingOwnersStrategy, MINIMUM_NON_BASE_FLAGS_SIZE, SINGLE_EPOCH_FLAGS_SIZE, +}; +use std::borrow::Cow; +use std::collections::BTreeMap; +use std::fmt; + +use self::StorageFlags::{ + MultiEpoch, MultiEpochContractBucket, MultiEpochOwned, SingleEpoch, SingleEpochContractBucket, + SingleEpochOwned, +}; + +/// An epoch index as stored in element flags +pub type EpochIndex = u16; + +/// The epoch in which an element was first stored +pub type BaseEpoch = EpochIndex; + +/// Bytes added to an element in a later epoch +pub type BytesAddedInEpoch = u32; + +/// The identity that owns stored bytes, as 32 raw bytes +pub type OwnerId = [u8; 32]; + +/// The contract whose credit bucket owns stored bytes, as 32 raw bytes +pub type ContractId = [u8; 32]; + +/// Size of the owner id carried by the identity-owned variants +const OWNER_ID_SIZE: u32 = 32; + +/// Size of the contract bucket owner: contract id plus two-byte position +const CONTRACT_BUCKET_OWNER_SIZE: u32 = 34; + +/// Storage flags: the epoch that paid for an element's bytes and, when +/// someone other than the system paid, the recorded owner. +/// +/// Variants 0 to 3 are encoded and decoded by the pinned +/// `grovedb_epoch_based_storage_flags` crate and are byte for byte what is on +/// chain today. Variants 4 and 5 carry a contract credit bucket owner. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum StorageFlags { + /// Single epoch, unowned + /// represented as byte 0 + SingleEpoch(BaseEpoch), + + /// Multi epoch, unowned + /// represented as byte 1 + MultiEpoch(BaseEpoch, BTreeMap), + + /// Single epoch owned by an identity + /// represented as byte 2 + SingleEpochOwned(BaseEpoch, OwnerId), + + /// Multi epoch owned by an identity + /// represented as byte 3 + MultiEpochOwned(BaseEpoch, BTreeMap, OwnerId), + + /// Single epoch owned by a contract credit bucket + /// represented as byte 4 + /// + /// Layout: type byte, contract id (32 bytes), bucket position (2 bytes, + /// big-endian), base epoch (2 bytes, big-endian); 37 bytes in total. + /// + /// Provisional: the type byte allocation is proposed in the fees + /// workstream register (issue 4689) and pending the owner's confirmation. + SingleEpochContractBucket(BaseEpoch, ContractId, ContractCreditBucketPosition), + + /// Multi epoch owned by a contract credit bucket + /// represented as byte 5 + /// + /// Layout: the 37-byte header of the single epoch bucket variant followed + /// by the crate's epoch map encoding (2-byte epoch index, varint bytes). + MultiEpochContractBucket( + BaseEpoch, + BTreeMap, + ContractId, + ContractCreditBucketPosition, + ), +} + +impl fmt::Display for StorageFlags { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + SingleEpoch(base_epoch) => { + write!(f, "SingleEpoch(BaseEpoch: {})", base_epoch) + } + MultiEpoch(base_epoch, epochs) => { + write!(f, "MultiEpoch(BaseEpoch: {}, Epochs: ", base_epoch)?; + for (index, bytes) in epochs { + write!(f, "[EpochIndex: {}, BytesAdded: {}] ", index, bytes)?; + } + write!(f, ")") + } + SingleEpochOwned(base_epoch, owner_id) => { + write!( + f, + "SingleEpochOwned(BaseEpoch: {}, OwnerId: {})", + base_epoch, + hex::encode(owner_id) + ) + } + MultiEpochOwned(base_epoch, epochs, owner_id) => { + write!(f, "MultiEpochOwned(BaseEpoch: {}, Epochs: ", base_epoch)?; + for (index, bytes) in epochs { + write!(f, "[EpochIndex: {}, BytesAdded: {}] ", index, bytes)?; + } + write!(f, ", OwnerId: {})", hex::encode(owner_id)) + } + SingleEpochContractBucket(base_epoch, contract_id, position) => { + write!( + f, + "SingleEpochContractBucket(BaseEpoch: {}, ContractId: {}, Position: {})", + base_epoch, + hex::encode(contract_id), + position + ) + } + MultiEpochContractBucket(base_epoch, epochs, contract_id, position) => { + write!( + f, + "MultiEpochContractBucket(BaseEpoch: {}, Epochs: ", + base_epoch + )?; + for (index, bytes) in epochs { + write!(f, "[EpochIndex: {}, BytesAdded: {}] ", index, bytes)?; + } + write!( + f, + ", ContractId: {}, Position: {})", + hex::encode(contract_id), + position + ) + } + } + } +} + +impl From for StorageFlags { + fn from(value: CrateStorageFlags) -> Self { + match value { + CrateStorageFlags::SingleEpoch(base_epoch) => SingleEpoch(base_epoch), + CrateStorageFlags::MultiEpoch(base_epoch, epochs) => MultiEpoch(base_epoch, epochs), + CrateStorageFlags::SingleEpochOwned(base_epoch, owner_id) => { + SingleEpochOwned(base_epoch, owner_id) + } + CrateStorageFlags::MultiEpochOwned(base_epoch, epochs, owner_id) => { + MultiEpochOwned(base_epoch, epochs, owner_id) + } + } + } +} + +impl StorageFlags { + /// Create new single epoch storage flags, owned by an identity when an + /// owner id is given + pub fn new_single_epoch(epoch: BaseEpoch, maybe_owner_id: Option) -> Self { + match maybe_owner_id { + None => SingleEpoch(epoch), + Some(owner_id) => SingleEpochOwned(epoch, owner_id), + } + } + + /// Create new single epoch storage flags for a typed owner, or unowned + /// flags when no owner is given + pub fn new_single_epoch_for_owner(epoch: BaseEpoch, owner: Option) -> Self { + match owner { + None => SingleEpoch(epoch), + Some(RefundOwner::Identity(identity_id)) => { + SingleEpochOwned(epoch, identity_id.to_buffer()) + } + Some(RefundOwner::ContractBucket { + contract_id, + position, + }) => SingleEpochContractBucket(epoch, contract_id.to_buffer(), position), + } + } + + /// Sets the owner id if we have identity-owned storage flags + pub fn set_owner_id(&mut self, owner_id: OwnerId) { + match self { + SingleEpochOwned(_, previous_owner_id) | MultiEpochOwned(_, _, previous_owner_id) => { + *previous_owner_id = owner_id; + } + _ => {} + } + } + + /// Returns base epoch + pub fn base_epoch(&self) -> &BaseEpoch { + match self { + SingleEpoch(base_epoch) + | MultiEpoch(base_epoch, _) + | SingleEpochOwned(base_epoch, _) + | MultiEpochOwned(base_epoch, ..) + | SingleEpochContractBucket(base_epoch, ..) + | MultiEpochContractBucket(base_epoch, ..) => base_epoch, + } + } + + /// Returns the owner id when an identity owns the bytes. + /// + /// Contract bucket owners are not identities and return `None` here; use + /// [`Self::refund_owner`] for the typed owner of any variant. + pub fn owner_id(&self) -> Option<&OwnerId> { + match self { + SingleEpochOwned(_, owner_id) | MultiEpochOwned(_, _, owner_id) => Some(owner_id), + _ => None, + } + } + + /// Returns the typed owner of the bytes, if anyone other than the system + /// paid for them + pub fn refund_owner(&self) -> Option { + match self { + SingleEpoch(_) | MultiEpoch(..) => None, + SingleEpochOwned(_, owner_id) | MultiEpochOwned(_, _, owner_id) => { + Some(RefundOwner::Identity(Identifier::from(*owner_id))) + } + SingleEpochContractBucket(_, contract_id, position) + | MultiEpochContractBucket(_, _, contract_id, position) => { + Some(RefundOwner::ContractBucket { + contract_id: Identifier::from(*contract_id), + position: *position, + }) + } + } + } + + /// Returns epoch index map + pub fn epoch_index_map(&self) -> Option<&BTreeMap> { + match self { + MultiEpoch(_, epoch_int_map) + | MultiEpochOwned(_, epoch_int_map, _) + | MultiEpochContractBucket(_, epoch_int_map, ..) => Some(epoch_int_map), + _ => None, + } + } + + /// Returns optional default storage flags + pub fn optional_default() -> Option { + None + } + + /// Returns default optional storage flag as ref + pub fn optional_default_as_ref() -> Option<&'static Self> { + None + } + + /// Returns default optional storage flag as ref + pub fn optional_default_as_cow() -> Option> { + None + } + + /// Returns type byte + pub fn type_byte(&self) -> u8 { + match self { + SingleEpoch(_) => 0, + MultiEpoch(..) => 1, + SingleEpochOwned(..) => 2, + MultiEpochOwned(..) => 3, + SingleEpochContractBucket(..) => 4, + MultiEpochContractBucket(..) => 5, + } + } + + /// Approximate serialized size of flags with or without an identity owner + pub fn approximate_size( + has_owner_id: bool, + approximate_changes_and_bytes_count: Option<(u16, u8)>, + ) -> u32 { + CrateStorageFlags::approximate_size(has_owner_id, approximate_changes_and_bytes_count) + } + + /// Approximate serialized size of flags for a typed owner: 3 bytes for + /// the type byte and base epoch, plus 32 for an identity owner or 34 for + /// a contract bucket owner, plus the approximate epoch map size + pub fn approximate_size_for_owner( + owner: Option<&RefundOwner>, + approximate_changes_and_bytes_count: Option<(u16, u8)>, + ) -> u32 { + let mut size = SINGLE_EPOCH_FLAGS_SIZE; + match owner { + None => {} + Some(RefundOwner::Identity(_)) => size += OWNER_ID_SIZE, + Some(RefundOwner::ContractBucket { .. }) => size += CONTRACT_BUCKET_OWNER_SIZE, + } + if let Some((approximate_change_count, bytes_changed_required_size)) = + approximate_changes_and_bytes_count + { + size += (approximate_change_count as u32) * (2 + bytes_changed_required_size as u32) + } + size + } + + /// Wrap Storage Flags into optional owned cow + pub fn into_optional_cow<'a>(self) -> Option> { + Some(Cow::Owned(self)) + } + + /// The crate's value with every typed owner replaced by its removal key. + /// + /// This is what lets the crate's epoch arithmetic (split and combine) run + /// unchanged over the bucket variants: the crate only ever compares owner + /// ids for equality and sections removed bytes under them, and the + /// removal key is exactly the key those bytes must be sectioned under. + /// Callers map the result back through [`Self::refund_owner`] of the + /// inputs so the kind is never read from the key. + pub(super) fn to_crate_flags_keyed_by_removal_key(&self) -> CrateStorageFlags { + match self { + SingleEpoch(base_epoch) => CrateStorageFlags::SingleEpoch(*base_epoch), + MultiEpoch(base_epoch, epochs) => { + CrateStorageFlags::MultiEpoch(*base_epoch, epochs.clone()) + } + SingleEpochOwned(base_epoch, owner_id) => { + CrateStorageFlags::SingleEpochOwned(*base_epoch, *owner_id) + } + MultiEpochOwned(base_epoch, epochs, owner_id) => { + CrateStorageFlags::MultiEpochOwned(*base_epoch, epochs.clone(), *owner_id) + } + SingleEpochContractBucket(base_epoch, contract_id, position) => { + CrateStorageFlags::SingleEpochOwned( + *base_epoch, + bucket_removal_key(contract_id, *position), + ) + } + MultiEpochContractBucket(base_epoch, epochs, contract_id, position) => { + CrateStorageFlags::MultiEpochOwned( + *base_epoch, + epochs.clone(), + bucket_removal_key(contract_id, *position), + ) + } + } + } +} + +/// The carrier key of a contract bucket owner +fn bucket_removal_key( + contract_id: &ContractId, + position: ContractCreditBucketPosition, +) -> [u8; 32] { + RefundOwner::ContractBucket { + contract_id: Identifier::from(*contract_id), + position, + } + .removal_key() +} + +#[cfg(test)] +mod tests; diff --git a/packages/rs-drive/src/util/storage_flags/split.rs b/packages/rs-drive/src/util/storage_flags/split.rs new file mode 100644 index 00000000000..4e810c139cf --- /dev/null +++ b/packages/rs-drive/src/util/storage_flags/split.rs @@ -0,0 +1,103 @@ +//! Splitting removed bytes across epochs and owners when an element shrinks +//! or is deleted. + +use super::{CrateStorageFlags, StorageFlags}; +use crate::error::drive::DriveError; +use crate::error::Error; +use dpp::fee::refund_owner::{RefundOwner, RefundOwnersByIdentifier, SYSTEM_REFUND_CARRIER_KEY}; +use grovedb::ElementFlags; +use grovedb_costs::storage_cost::removal::StorageRemovedBytes; +use grovedb_costs::storage_cost::removal::StorageRemovedBytes::BasicStorageRemoval; +use grovedb_epoch_based_storage_flags::error::StorageFlagsError; + +impl StorageFlags { + /// Sections removed bytes per epoch, taking from the latest epochs + /// first, under the owner's carrier key (or the system key when the + /// bytes are unowned). + /// + /// The carrier key alone does not say what kind of owner it names; use + /// [`Self::split_removal_bytes_typed`] on a path that must route the + /// refund, so that the owner is recorded alongside. + pub fn split_storage_removed_bytes( + &self, + removed_key_bytes: u32, + removed_value_bytes: u32, + ) -> (StorageRemovedBytes, StorageRemovedBytes) { + self.to_crate_flags_keyed_by_removal_key() + .split_storage_removed_bytes(removed_key_bytes, removed_value_bytes) + } + + /// The batch split closure that shipped with identity-only owners. + /// + /// Hands the raw bytes to the crate, so a contract bucket type byte is + /// rejected as an unknown flags type: the batch apply generation that + /// predates typed owners cannot price bytes it cannot attribute. + pub fn split_removal_bytes( + flags: &mut ElementFlags, + removed_key_bytes: u32, + removed_value_bytes: u32, + ) -> Result<(StorageRemovedBytes, StorageRemovedBytes), StorageFlagsError> { + CrateStorageFlags::split_removal_bytes(flags, removed_key_bytes, removed_value_bytes) + } + + /// The batch split closure for typed owners. + /// + /// Decodes all six flag types, sections the removed bytes under the + /// owner's carrier key and records `(carrier key, owner)` in + /// `refund_owners` for every owned removal, identities included. Within + /// one batch a carrier key may name only one owner; a second owner for + /// the same key, or a bucket whose key equals the system key, fails the + /// batch. Nothing is guessed and nothing is re-derived. + pub fn split_removal_bytes_typed( + flags: &mut ElementFlags, + removed_key_bytes: u32, + removed_value_bytes: u32, + refund_owners: &mut RefundOwnersByIdentifier, + ) -> Result<(StorageRemovedBytes, StorageRemovedBytes), Error> { + let maybe_storage_flags = Self::from_element_flags_ref(flags)?; + match maybe_storage_flags { + None => Ok(( + BasicStorageRemoval(removed_key_bytes), + BasicStorageRemoval(removed_value_bytes), + )), + Some(storage_flags) => { + if let Some(owner) = storage_flags.refund_owner() { + Self::record_refund_owner(owner, refund_owners)?; + } + Ok(storage_flags + .split_storage_removed_bytes(removed_key_bytes, removed_value_bytes)) + } + } + } + + fn record_refund_owner( + owner: RefundOwner, + refund_owners: &mut RefundOwnersByIdentifier, + ) -> Result<(), Error> { + let key = owner.removal_key(); + if key == SYSTEM_REFUND_CARRIER_KEY { + return match owner { + // The all-zero identity has always been sectioned as system + // bytes and never refunded; recording nothing keeps that. + RefundOwner::Identity(_) => Ok(()), + RefundOwner::ContractBucket { .. } => { + Err(Error::Drive(DriveError::CorruptedCodeExecution( + "a contract bucket refund owner derived the system carrier key", + ))) + } + }; + } + match refund_owners.get(&key) { + Some(existing) if *existing != owner => { + Err(Error::Drive(DriveError::CorruptedCodeExecution( + "two different refund owners share one storage removal carrier key within a batch", + ))) + } + Some(_) => Ok(()), + None => { + refund_owners.insert(key, owner); + Ok(()) + } + } + } +} diff --git a/packages/rs-drive/src/util/storage_flags/tests.rs b/packages/rs-drive/src/util/storage_flags/tests.rs new file mode 100644 index 00000000000..99d74837eb0 --- /dev/null +++ b/packages/rs-drive/src/util/storage_flags/tests.rs @@ -0,0 +1,773 @@ +//! Tests for the Drive storage flags: codec, split, combine and the batch +//! closure entry points. + +use super::*; +use crate::error::drive::DriveError; +use crate::error::Error; +use dpp::fee::refund_owner::{RefundOwnersByIdentifier, SYSTEM_REFUND_CARRIER_KEY}; +use grovedb_costs::storage_cost::removal::StorageRemovalPerEpochByIdentifier; +use grovedb_costs::storage_cost::removal::StorageRemovedBytes; +use grovedb_costs::storage_cost::removal::StorageRemovedBytes::{ + BasicStorageRemoval, NoStorageRemoval, SectionedStorageRemoval, +}; +use grovedb_costs::storage_cost::StorageCost; +use grovedb_epoch_based_storage_flags::error::StorageFlagsError; +use intmap::IntMap; + +const CONTRACT_ID: ContractId = [0x11; 32]; +const OWNER_ID: OwnerId = [0x22; 32]; + +fn bucket_owner(position: ContractCreditBucketPosition) -> RefundOwner { + RefundOwner::ContractBucket { + contract_id: Identifier::from(CONTRACT_ID), + position, + } +} + +fn epochs(entries: &[(EpochIndex, BytesAddedInEpoch)]) -> BTreeMap { + entries.iter().copied().collect() +} + +fn every_variant() -> Vec { + vec![ + SingleEpoch(3), + MultiEpoch(3, epochs(&[(4, 10), (5, 300)])), + SingleEpochOwned(3, OWNER_ID), + MultiEpochOwned(3, epochs(&[(4, 10), (5, 300)]), OWNER_ID), + SingleEpochContractBucket(3, CONTRACT_ID, 7), + MultiEpochContractBucket(3, epochs(&[(4, 10), (5, 300)]), CONTRACT_ID, 7), + MultiEpochContractBucket(0, epochs(&[(65535, u32::MAX)]), CONTRACT_ID, u16::MAX), + ] +} + +fn sectioned(key: [u8; 32], entries: &[(u16, u32)]) -> StorageRemovedBytes { + let mut map = StorageRemovalPerEpochByIdentifier::new(); + map.insert(key, IntMap::from_iter(entries.iter().copied())); + SectionedStorageRemoval(map) +} + +mod codec { + use super::*; + + #[test] + fn should_round_trip_every_variant() { + for flags in every_variant() { + let bytes = flags.serialize(); + let decoded = StorageFlags::deserialize(&bytes) + .expect("should decode") + .expect("should be some"); + assert_eq!(decoded, flags, "{}", flags); + assert_eq!(bytes.first().copied(), Some(flags.type_byte())); + } + } + + #[test] + fn should_encode_the_historical_variants_exactly_as_the_crate() { + let pairs = [ + (SingleEpoch(3), CrateStorageFlags::SingleEpoch(3)), + ( + MultiEpoch(3, epochs(&[(4, 10)])), + CrateStorageFlags::MultiEpoch(3, epochs(&[(4, 10)])), + ), + ( + SingleEpochOwned(3, OWNER_ID), + CrateStorageFlags::SingleEpochOwned(3, OWNER_ID), + ), + ( + MultiEpochOwned(3, epochs(&[(4, 10), (5, 300)]), OWNER_ID), + CrateStorageFlags::MultiEpochOwned(3, epochs(&[(4, 10), (5, 300)]), OWNER_ID), + ), + ]; + for (ours, theirs) in pairs { + assert_eq!(ours.serialize(), theirs.serialize(), "{}", ours); + assert_eq!(ours.serialized_size(), theirs.serialized_size()); + assert_eq!( + StorageFlags::deserialize(&theirs.serialize()).expect("should decode"), + Some(ours) + ); + } + } + + #[test] + fn should_pin_the_byte_layout_of_every_variant() { + assert_eq!(SingleEpoch(0x0102).serialize(), vec![0, 1, 2]); + assert_eq!( + MultiEpoch(0x0102, epochs(&[(0x0304, 300)])).serialize(), + vec![1, 1, 2, 3, 4, 0xac, 0x02] + ); + + let mut owned = vec![2]; + owned.extend_from_slice(&OWNER_ID); + owned.extend_from_slice(&[1, 2]); + assert_eq!(SingleEpochOwned(0x0102, OWNER_ID).serialize(), owned); + assert_eq!(owned.len(), 35); + + let mut multi_owned = vec![3]; + multi_owned.extend_from_slice(&OWNER_ID); + multi_owned.extend_from_slice(&[1, 2, 3, 4, 0xac, 0x02]); + assert_eq!( + MultiEpochOwned(0x0102, epochs(&[(0x0304, 300)]), OWNER_ID).serialize(), + multi_owned + ); + + let mut bucket = vec![4]; + bucket.extend_from_slice(&CONTRACT_ID); + bucket.extend_from_slice(&[0, 7]); + bucket.extend_from_slice(&[1, 2]); + assert_eq!( + SingleEpochContractBucket(0x0102, CONTRACT_ID, 7).serialize(), + bucket + ); + assert_eq!(bucket.len(), 37); + + let mut multi_bucket = vec![5]; + multi_bucket.extend_from_slice(&CONTRACT_ID); + multi_bucket.extend_from_slice(&[0, 7]); + multi_bucket.extend_from_slice(&[1, 2, 3, 4, 0xac, 0x02]); + assert_eq!( + MultiEpochContractBucket(0x0102, epochs(&[(0x0304, 300)]), CONTRACT_ID, 7).serialize(), + multi_bucket + ); + } + + #[test] + fn should_report_the_serialized_size_of_every_variant() { + for flags in every_variant() { + assert_eq!( + flags.serialized_size() as usize, + flags.serialize().len(), + "{}", + flags + ); + } + } + + #[test] + fn should_reject_an_unknown_type_byte() { + let mut bytes = vec![6]; + bytes.extend_from_slice(&[0; 36]); + let error = StorageFlags::deserialize(&bytes).expect_err("should reject"); + assert!(matches!( + error, + StorageFlagsError::DeserializeUnknownStorageFlagsType(_) + )); + assert!(matches!( + StorageFlags::deserialize(&[255, 1, 2]), + Err(StorageFlagsError::DeserializeUnknownStorageFlagsType(_)) + )); + } + + #[test] + fn should_reject_truncated_and_oversized_bucket_flags() { + let single = SingleEpochContractBucket(3, CONTRACT_ID, 7).serialize(); + for cut in [1usize, 32, 33, 34, 35, 36] { + assert!( + matches!( + StorageFlags::deserialize(&single[..cut]), + Err(StorageFlagsError::StorageFlagsWrongSize(_)) + ), + "single bucket cut at {}", + cut + ); + } + let mut too_long = single.clone(); + too_long.push(0); + assert!(matches!( + StorageFlags::deserialize(&too_long), + Err(StorageFlagsError::StorageFlagsWrongSize(_)) + )); + + let multi = MultiEpochContractBucket(3, epochs(&[(4, 300)]), CONTRACT_ID, 7).serialize(); + // header only, header plus one epoch byte, and a cut inside the varint + for cut in [37usize, 38, 39, multi.len() - 1] { + assert!( + matches!( + StorageFlags::deserialize(&multi[..cut]), + Err(StorageFlagsError::StorageFlagsWrongSize(_)) + ), + "multi bucket cut at {}", + cut + ); + } + // a dangling epoch index with no byte count + let mut dangling = multi.clone(); + dangling.extend_from_slice(&[0, 9]); + assert!(matches!( + StorageFlags::deserialize(&dangling), + Err(StorageFlagsError::StorageFlagsWrongSize(_)) + )); + } + + #[test] + fn should_decode_empty_flags_as_none() { + assert_eq!(StorageFlags::deserialize(&[]).expect("should decode"), None); + assert_eq!( + StorageFlags::map_some_element_flags_ref(&None).expect("should decode"), + None + ); + } + + #[test] + fn should_size_typed_owners_for_estimation() { + let identity = RefundOwner::Identity(Identifier::from(OWNER_ID)); + let bucket = bucket_owner(7); + + assert_eq!( + StorageFlags::approximate_size_for_owner(None, None), + SingleEpoch(0).serialized_size() + ); + assert_eq!( + StorageFlags::approximate_size_for_owner(Some(&identity), None), + SingleEpochOwned(0, OWNER_ID).serialized_size() + ); + assert_eq!( + StorageFlags::approximate_size_for_owner(Some(&bucket), None), + SingleEpochContractBucket(0, CONTRACT_ID, 7).serialized_size() + ); + assert_eq!( + StorageFlags::approximate_size_for_owner(Some(&identity), Some((2, 1))), + StorageFlags::approximate_size(true, Some((2, 1))) + ); + assert_eq!( + StorageFlags::approximate_size_for_owner(Some(&bucket), Some((2, 1))), + 37 + 2 * 3 + ); + } + + #[test] + fn should_expose_the_typed_owner_of_every_variant() { + let identity = RefundOwner::Identity(Identifier::from(OWNER_ID)); + let bucket = bucket_owner(7); + + assert_eq!(SingleEpoch(1).refund_owner(), None); + assert_eq!(MultiEpoch(1, epochs(&[(2, 3)])).refund_owner(), None); + assert_eq!(SingleEpochOwned(1, OWNER_ID).refund_owner(), Some(identity)); + assert_eq!( + MultiEpochOwned(1, epochs(&[(2, 3)]), OWNER_ID).refund_owner(), + Some(identity) + ); + assert_eq!( + SingleEpochContractBucket(1, CONTRACT_ID, 7).refund_owner(), + Some(bucket) + ); + assert_eq!( + MultiEpochContractBucket(1, epochs(&[(2, 3)]), CONTRACT_ID, 7).refund_owner(), + Some(bucket) + ); + + // the identity accessor never reads a bucket as an identity + assert_eq!( + SingleEpochContractBucket(1, CONTRACT_ID, 7).owner_id(), + None + ); + assert_eq!(SingleEpochOwned(1, OWNER_ID).owner_id(), Some(&OWNER_ID)); + + assert_eq!( + StorageFlags::new_single_epoch_for_owner(9, Some(bucket)), + SingleEpochContractBucket(9, CONTRACT_ID, 7) + ); + assert_eq!( + StorageFlags::new_single_epoch_for_owner(9, Some(identity)), + SingleEpochOwned(9, OWNER_ID) + ); + assert_eq!( + StorageFlags::new_single_epoch_for_owner(9, None), + SingleEpoch(9) + ); + } +} + +mod split { + use super::*; + + #[test] + fn should_section_a_deleted_bucket_element_under_its_carrier_key_and_record_the_owner() { + let owner = bucket_owner(7); + let mut flags = SingleEpochContractBucket(5, CONTRACT_ID, 7).to_element_flags(); + let mut refund_owners = RefundOwnersByIdentifier::new(); + + let (key_removal, value_removal) = + StorageFlags::split_removal_bytes_typed(&mut flags, 50, 150, &mut refund_owners) + .expect("should split"); + + assert_eq!(key_removal, sectioned(owner.removal_key(), &[(5, 50)])); + assert_eq!(value_removal, sectioned(owner.removal_key(), &[(5, 150)])); + assert_eq!( + refund_owners, + RefundOwnersByIdentifier::from([(owner.removal_key(), owner)]) + ); + } + + #[test] + fn should_take_from_the_latest_epochs_first_when_a_bucket_element_shrinks() { + let owner = bucket_owner(7); + let flags = MultiEpochContractBucket(5, epochs(&[(6, 300), (7, 400)]), CONTRACT_ID, 7); + let mut refund_owners = RefundOwnersByIdentifier::new(); + + let (key_removal, value_removal) = StorageFlags::split_removal_bytes_typed( + &mut flags.to_element_flags(), + 0, + 700, + &mut refund_owners, + ) + .expect("should split"); + + assert_eq!(key_removal, NoStorageRemoval); + // the same LIFO sectioning the crate applies to identity-owned flags + assert_eq!( + value_removal, + sectioned(owner.removal_key(), &[(5, 6), (6, 297), (7, 397)]) + ); + assert_eq!(refund_owners.get(&owner.removal_key()), Some(&owner)); + + let crate_equivalent = CrateStorageFlags::MultiEpochOwned( + 5, + epochs(&[(6, 300), (7, 400)]), + owner.removal_key(), + ) + .split_storage_removed_bytes(0, 700); + assert_eq!(value_removal, crate_equivalent.1); + } + + #[test] + fn should_record_an_identity_owner() { + let owner = RefundOwner::Identity(Identifier::from(OWNER_ID)); + let mut refund_owners = RefundOwnersByIdentifier::new(); + + let (key_removal, value_removal) = StorageFlags::split_removal_bytes_typed( + &mut SingleEpochOwned(2, OWNER_ID).to_element_flags(), + 10, + 20, + &mut refund_owners, + ) + .expect("should split"); + + assert_eq!(key_removal, sectioned(OWNER_ID, &[(2, 10)])); + assert_eq!(value_removal, sectioned(OWNER_ID, &[(2, 20)])); + assert_eq!( + refund_owners, + RefundOwnersByIdentifier::from([(OWNER_ID, owner)]) + ); + } + + #[test] + fn should_section_unowned_bytes_under_the_system_key_and_record_nothing() { + let mut refund_owners = RefundOwnersByIdentifier::new(); + + let (key_removal, value_removal) = StorageFlags::split_removal_bytes_typed( + &mut SingleEpoch(2).to_element_flags(), + 10, + 20, + &mut refund_owners, + ) + .expect("should split"); + + assert_eq!( + key_removal, + sectioned(SYSTEM_REFUND_CARRIER_KEY, &[(2, 10)]) + ); + assert_eq!( + value_removal, + sectioned(SYSTEM_REFUND_CARRIER_KEY, &[(2, 20)]) + ); + assert!(refund_owners.is_empty()); + + let (key_removal, value_removal) = + StorageFlags::split_removal_bytes_typed(&mut vec![], 10, 20, &mut refund_owners) + .expect("should split"); + assert_eq!(key_removal, BasicStorageRemoval(10)); + assert_eq!(value_removal, BasicStorageRemoval(20)); + assert!(refund_owners.is_empty()); + } + + #[test] + fn should_keep_the_all_zero_identity_as_system_bytes_and_record_nothing() { + let mut refund_owners = RefundOwnersByIdentifier::new(); + + let (key_removal, _) = StorageFlags::split_removal_bytes_typed( + &mut SingleEpochOwned(2, SYSTEM_REFUND_CARRIER_KEY).to_element_flags(), + 10, + 20, + &mut refund_owners, + ) + .expect("should split"); + + assert_eq!( + key_removal, + sectioned(SYSTEM_REFUND_CARRIER_KEY, &[(2, 10)]) + ); + assert!(refund_owners.is_empty()); + } + + #[test] + fn should_reject_one_carrier_key_recorded_for_two_owners_within_a_batch() { + let owner = bucket_owner(7); + let mut refund_owners = RefundOwnersByIdentifier::from([( + owner.removal_key(), + RefundOwner::Identity(Identifier::from(owner.removal_key())), + )]); + + let result = StorageFlags::split_removal_bytes_typed( + &mut SingleEpochContractBucket(5, CONTRACT_ID, 7).to_element_flags(), + 1, + 1, + &mut refund_owners, + ); + + assert!(matches!( + result, + Err(Error::Drive(DriveError::CorruptedCodeExecution(_))) + )); + } + + #[test] + fn should_keep_the_same_owner_recorded_once_across_two_elements() { + let owner = bucket_owner(7); + let mut refund_owners = RefundOwnersByIdentifier::new(); + for _ in 0..2 { + StorageFlags::split_removal_bytes_typed( + &mut SingleEpochContractBucket(5, CONTRACT_ID, 7).to_element_flags(), + 1, + 1, + &mut refund_owners, + ) + .expect("should split"); + } + assert_eq!(refund_owners.len(), 1); + assert_eq!(refund_owners.get(&owner.removal_key()), Some(&owner)); + } + + #[test] + fn should_fail_closed_on_bucket_flags_through_the_shipped_closure_entry_points() { + let mut flags = SingleEpochContractBucket(5, CONTRACT_ID, 7).to_element_flags(); + assert!(matches!( + StorageFlags::split_removal_bytes(&mut flags, 1, 1), + Err(StorageFlagsError::DeserializeUnknownStorageFlagsType(_)) + )); + + let cost = StorageCost { + added_bytes: 10, + replaced_bytes: 1, + removed_bytes: NoStorageRemoval, + }; + let mut new_flags = SingleEpochContractBucket(6, CONTRACT_ID, 7).to_element_flags(); + assert!(matches!( + StorageFlags::update_element_flags( + &cost, + Some(SingleEpochContractBucket(5, CONTRACT_ID, 7).to_element_flags()), + &mut new_flags + ), + Err(StorageFlagsError::DeserializeUnknownStorageFlagsType(_)) + )); + + // the shipped entry points still work for the historical variants + let mut owned = SingleEpochOwned(5, OWNER_ID).to_element_flags(); + let (key_removal, _) = + StorageFlags::split_removal_bytes(&mut owned, 3, 4).expect("should split"); + assert_eq!(key_removal, sectioned(OWNER_ID, &[(5, 3)])); + } +} + +mod combine { + use super::*; + + #[test] + fn should_keep_the_bucket_owner_when_combining_the_same_base_epoch() { + let ours = SingleEpochContractBucket(1, CONTRACT_ID, 7); + let theirs = SingleEpochContractBucket(1, CONTRACT_ID, 7); + + let combined = ours + .combine_added_bytes(theirs, 10, MergingOwnersStrategy::RaiseIssue) + .expect("should combine"); + + assert_eq!(combined, SingleEpochContractBucket(1, CONTRACT_ID, 7)); + } + + #[test] + fn should_transfer_ownership_across_kinds_with_use_theirs() { + let identity_to_bucket = SingleEpochOwned(1, OWNER_ID) + .combine_added_bytes( + SingleEpochContractBucket(1, CONTRACT_ID, 7), + 10, + MergingOwnersStrategy::UseTheirs, + ) + .expect("should combine"); + assert_eq!( + identity_to_bucket, + SingleEpochContractBucket(1, CONTRACT_ID, 7) + ); + + let bucket_to_identity = SingleEpochContractBucket(1, CONTRACT_ID, 7) + .combine_added_bytes( + SingleEpochOwned(1, OWNER_ID), + 10, + MergingOwnersStrategy::UseTheirs, + ) + .expect("should combine"); + assert_eq!(bucket_to_identity, SingleEpochOwned(1, OWNER_ID)); + + let kept_ours = SingleEpochContractBucket(1, CONTRACT_ID, 7) + .combine_added_bytes( + SingleEpochOwned(1, OWNER_ID), + 10, + MergingOwnersStrategy::UseOurs, + ) + .expect("should combine"); + assert_eq!(kept_ours, SingleEpochContractBucket(1, CONTRACT_ID, 7)); + } + + #[test] + fn should_raise_an_issue_across_kinds_and_across_buckets() { + assert!(matches!( + SingleEpochOwned(1, OWNER_ID).combine_added_bytes( + SingleEpochContractBucket(1, CONTRACT_ID, 7), + 10, + MergingOwnersStrategy::RaiseIssue, + ), + Err(StorageFlagsError::MergingStorageFlagsFromDifferentOwners(_)) + )); + assert!(matches!( + SingleEpochContractBucket(1, CONTRACT_ID, 7).combine_added_bytes( + SingleEpochContractBucket(1, CONTRACT_ID, 8), + 10, + MergingOwnersStrategy::RaiseIssue, + ), + Err(StorageFlagsError::MergingStorageFlagsFromDifferentOwners(_)) + )); + } + + #[test] + fn should_never_merge_an_identity_whose_id_equals_a_bucket_carrier_key() { + let bucket = bucket_owner(7); + let colliding_identity = SingleEpochOwned(1, bucket.removal_key()); + + let result = colliding_identity.combine_added_bytes( + SingleEpochContractBucket(1, CONTRACT_ID, 7), + 10, + MergingOwnersStrategy::UseTheirs, + ); + + assert!(matches!( + result, + Err(StorageFlagsError::MergingStorageFlagsFromDifferentOwners(_)) + )); + } + + #[test] + fn should_keep_the_typed_owner_when_adding_bytes_in_a_higher_epoch() { + let combined = SingleEpochContractBucket(1, CONTRACT_ID, 7) + .combine_added_bytes( + SingleEpochContractBucket(2, CONTRACT_ID, 7), + 10, + MergingOwnersStrategy::UseTheirs, + ) + .expect("should combine"); + assert_eq!( + combined, + MultiEpochContractBucket(1, epochs(&[(2, 10)]), CONTRACT_ID, 7) + ); + + let again = combined + .combine_added_bytes( + SingleEpochContractBucket(2, CONTRACT_ID, 7), + 5, + MergingOwnersStrategy::UseTheirs, + ) + .expect("should combine"); + assert_eq!( + again, + MultiEpochContractBucket(1, epochs(&[(2, 15)]), CONTRACT_ID, 7) + ); + + // same arithmetic as the crate performs for an identity owner + let crate_combined = CrateStorageFlags::SingleEpochOwned(1, OWNER_ID) + .combine_added_bytes( + CrateStorageFlags::SingleEpochOwned(2, OWNER_ID), + 10, + MergingOwnersStrategy::UseTheirs, + ) + .expect("should combine"); + assert_eq!( + crate_combined, + CrateStorageFlags::MultiEpochOwned(1, epochs(&[(2, 10)]), OWNER_ID) + ); + } + + #[test] + fn should_keep_the_typed_owner_when_removing_bytes_in_a_higher_epoch() { + let owner = bucket_owner(7); + let flags = MultiEpochContractBucket(1, epochs(&[(2, 20)]), CONTRACT_ID, 7); + + let removed = sectioned(owner.removal_key(), &[(2, 5)]); + let combined = flags + .clone() + .combine_removed_bytes( + SingleEpochContractBucket(2, CONTRACT_ID, 7), + &removed, + MergingOwnersStrategy::UseTheirs, + ) + .expect("should combine"); + assert_eq!( + combined, + MultiEpochContractBucket(1, epochs(&[(2, 15)]), CONTRACT_ID, 7) + ); + + // removing the whole epoch collapses back to the single epoch variant + let removed_all = sectioned(owner.removal_key(), &[(2, 18)]); + let collapsed = flags + .combine_removed_bytes( + SingleEpochContractBucket(2, CONTRACT_ID, 7), + &removed_all, + MergingOwnersStrategy::UseTheirs, + ) + .expect("should combine"); + assert_eq!(collapsed, SingleEpochContractBucket(1, CONTRACT_ID, 7)); + } + + #[test] + fn should_reject_a_newer_base_epoch_merging_into_an_older_one() { + assert!(matches!( + SingleEpochContractBucket(2, CONTRACT_ID, 7).combine_added_bytes( + SingleEpochContractBucket(1, CONTRACT_ID, 7), + 10, + MergingOwnersStrategy::UseTheirs, + ), + Err(StorageFlagsError::MergingStorageFlagsWithDifferentBaseEpoch(_)) + )); + } +} + +mod update { + use super::*; + + fn bigger(added_bytes: u32) -> StorageCost { + StorageCost { + added_bytes, + replaced_bytes: 1, + removed_bytes: NoStorageRemoval, + } + } + + #[test] + fn should_behave_like_the_crate_for_identity_owned_flags() { + let old = SingleEpochOwned(1, OWNER_ID).to_element_flags(); + let mut typed_new = SingleEpochOwned(2, OWNER_ID).to_element_flags(); + let mut crate_new = typed_new.clone(); + + let typed_changed = StorageFlags::update_element_flags_typed( + &bigger(10), + Some(old.clone()), + &mut typed_new, + ) + .expect("should update"); + let crate_changed = + CrateStorageFlags::update_element_flags(&bigger(10), Some(old), &mut crate_new) + .expect("should update"); + + assert_eq!(typed_changed, crate_changed); + assert_eq!(typed_new, crate_new); + assert_eq!( + StorageFlags::deserialize(&typed_new).expect("should decode"), + Some(MultiEpochOwned(1, epochs(&[(2, 10)]), OWNER_ID)) + ); + } + + #[test] + fn should_grow_a_bucket_owned_element_into_a_multi_epoch_bucket() { + let old = SingleEpochContractBucket(1, CONTRACT_ID, 7).to_element_flags(); + let mut new_flags = SingleEpochContractBucket(2, CONTRACT_ID, 7).to_element_flags(); + + let changed = + StorageFlags::update_element_flags_typed(&bigger(10), Some(old), &mut new_flags) + .expect("should update"); + + assert!(changed); + assert_eq!( + StorageFlags::deserialize(&new_flags).expect("should decode"), + Some(MultiEpochContractBucket( + 1, + epochs(&[(2, 10)]), + CONTRACT_ID, + 7 + )) + ); + } + + #[test] + fn should_transfer_a_replaced_element_to_the_new_typed_owner() { + let old = SingleEpochOwned(1, OWNER_ID).to_element_flags(); + let mut new_flags = SingleEpochContractBucket(1, CONTRACT_ID, 7).to_element_flags(); + + let changed = + StorageFlags::update_element_flags_typed(&bigger(10), Some(old), &mut new_flags) + .expect("should update"); + + assert!(!changed, "new flags already name the new owner"); + assert_eq!( + StorageFlags::deserialize(&new_flags).expect("should decode"), + Some(SingleEpochContractBucket(1, CONTRACT_ID, 7)) + ); + } + + #[test] + fn should_shrink_a_bucket_owned_element_against_its_sectioned_removal() { + let owner = bucket_owner(7); + let old = + MultiEpochContractBucket(1, epochs(&[(2, 20)]), CONTRACT_ID, 7).to_element_flags(); + let mut new_flags = SingleEpochContractBucket(2, CONTRACT_ID, 7).to_element_flags(); + let cost = StorageCost { + added_bytes: 0, + replaced_bytes: 1, + removed_bytes: sectioned(owner.removal_key(), &[(2, 5)]), + }; + + let changed = StorageFlags::update_element_flags_typed(&cost, Some(old), &mut new_flags) + .expect("should update"); + + assert!(changed); + assert_eq!( + StorageFlags::deserialize(&new_flags).expect("should decode"), + Some(MultiEpochContractBucket( + 1, + epochs(&[(2, 15)]), + CONTRACT_ID, + 7 + )) + ); + } + + #[test] + fn should_keep_old_flags_on_a_same_size_update_and_pass_through_inserts() { + let old = SingleEpochContractBucket(9, CONTRACT_ID, 7).to_element_flags(); + let mut new_flags = SingleEpochContractBucket(1, CONTRACT_ID, 7).to_element_flags(); + let same_size = StorageCost { + added_bytes: 0, + replaced_bytes: 1, + removed_bytes: NoStorageRemoval, + }; + let changed = + StorageFlags::update_element_flags_typed(&same_size, Some(old.clone()), &mut new_flags) + .expect("should update"); + assert!(changed); + assert_eq!(new_flags, old); + + let mut inserted = SingleEpochContractBucket(1, CONTRACT_ID, 7).to_element_flags(); + let changed = StorageFlags::update_element_flags_typed(&bigger(10), None, &mut inserted) + .expect("should update"); + assert!(!changed); + } + + #[test] + fn should_reject_removing_flags_from_a_flagged_element() { + let old = SingleEpochContractBucket(1, CONTRACT_ID, 7).to_element_flags(); + let mut new_flags = vec![]; + let result = + StorageFlags::update_element_flags_typed(&bigger(10), Some(old), &mut new_flags); + assert!(matches!( + result, + Err(Error::StorageFlags(StorageFlagsError::RemovingFlagsError( + _ + ))) + )); + } +} diff --git a/packages/rs-drive/src/util/storage_flags/update.rs b/packages/rs-drive/src/util/storage_flags/update.rs new file mode 100644 index 00000000000..3c621914034 --- /dev/null +++ b/packages/rs-drive/src/util/storage_flags/update.rs @@ -0,0 +1,88 @@ +//! Updating the flags of an element that is replaced in a batch. + +use super::{CrateStorageFlags, MergingOwnersStrategy, StorageFlags}; +use crate::error::Error; +use grovedb::ElementFlags; +use grovedb_costs::storage_cost::transition::OperationStorageTransitionType; +use grovedb_costs::storage_cost::StorageCost; +use grovedb_epoch_based_storage_flags::error::StorageFlagsError; + +impl StorageFlags { + /// The batch flag update closure that shipped with identity-only owners. + /// + /// Hands the raw bytes to the crate, so a contract bucket type byte is + /// rejected as an unknown flags type. + pub fn update_element_flags( + cost: &StorageCost, + old_flags: Option, + new_flags: &mut ElementFlags, + ) -> Result { + CrateStorageFlags::update_element_flags(cost, old_flags, new_flags) + } + + /// The batch flag update closure for typed owners: the crate's rule + /// expressed over the typed flags. + /// + /// A replace merges the old flags into the new ones with `UseTheirs`, so + /// new flags naming a different owner transfer the bytes to that owner. + /// That holds across kinds: an identity can hand bytes to a contract + /// bucket and a bucket to an identity. + pub fn update_element_flags_typed( + cost: &StorageCost, + old_flags: Option, + new_flags: &mut ElementFlags, + ) -> Result { + // if there were no flags before then the new flags are used + let Some(old_flags) = old_flags else { + return Ok(false); + }; + + let maybe_old_storage_flags = Self::from_element_flags_ref(&old_flags)?; + let new_storage_flags = Self::from_element_flags_ref(new_flags)?.ok_or( + StorageFlagsError::RemovingFlagsError( + "removing flags from an item with flags is not allowed".to_string(), + ), + )?; + let Some(old_storage_flags) = maybe_old_storage_flags else { + return Err(StorageFlagsError::RemovingFlagsError( + "old storage flags missing during update".to_string(), + ) + .into()); + }; + + match &cost.transition_type() { + OperationStorageTransitionType::OperationUpdateBiggerSize => { + let combined_storage_flags = old_storage_flags.combine_added_bytes( + new_storage_flags, + cost.added_bytes, + MergingOwnersStrategy::UseTheirs, + )?; + Ok(Self::replace_if_changed(combined_storage_flags, new_flags)) + } + OperationStorageTransitionType::OperationUpdateSmallerSize => { + let combined_storage_flags = old_storage_flags.combine_removed_bytes( + new_storage_flags, + &cost.removed_bytes, + MergingOwnersStrategy::UseTheirs, + )?; + Ok(Self::replace_if_changed(combined_storage_flags, new_flags)) + } + OperationStorageTransitionType::OperationUpdateSameSize => { + // a same-size update keeps the old flags + *new_flags = old_storage_flags.to_element_flags(); + Ok(true) + } + _ => Ok(false), + } + } + + fn replace_if_changed(combined: StorageFlags, new_flags: &mut ElementFlags) -> bool { + let combined_flags = combined.to_element_flags(); + if combined_flags == *new_flags { + false + } else { + *new_flags = combined_flags; + true + } + } +} diff --git a/packages/rs-drive/src/verify/contract/verify_contracts_by_range/v0/mod.rs b/packages/rs-drive/src/verify/contract/verify_contracts_by_range/v0/mod.rs index 70e7a343274..983bcaab478 100644 --- a/packages/rs-drive/src/verify/contract/verify_contracts_by_range/v0/mod.rs +++ b/packages/rs-drive/src/verify/contract/verify_contracts_by_range/v0/mod.rs @@ -196,6 +196,7 @@ fn identifier_from_bytes(bytes: Vec) -> Result { mod tests { use crate::drive::Drive; use crate::error::Error; + use crate::util::storage_flags::StorageFlags; use crate::util::test_helpers::setup::setup_drive_with_initial_state_structure; use dpp::block::block_info::BlockInfo; use dpp::data_contract::accessors::v0::{DataContractV0Getters, DataContractV0Setters}; @@ -206,7 +207,6 @@ mod tests { use dpp::prelude::Identifier; use dpp::tests::json_document::json_document_to_contract; use dpp::version::PlatformVersion; - use grovedb_epoch_based_storage_flags::StorageFlags; const PLAIN_CONTRACT: &str = "tests/supporting_files/contract/family/family-contract.json"; const HISTORY_CONTRACT: &str = diff --git a/packages/rs-drive/src/verify/contract/verify_contracts_versions/v0/mod.rs b/packages/rs-drive/src/verify/contract/verify_contracts_versions/v0/mod.rs index c75af2651d0..27a0068abc2 100644 --- a/packages/rs-drive/src/verify/contract/verify_contracts_versions/v0/mod.rs +++ b/packages/rs-drive/src/verify/contract/verify_contracts_versions/v0/mod.rs @@ -147,6 +147,7 @@ fn contract_id_from_bytes(bytes: &[u8]) -> Result<[u8; 32], Error> { mod tests { use crate::drive::Drive; use crate::error::Error; + use crate::util::storage_flags::StorageFlags; use crate::util::test_helpers::setup::setup_drive_with_initial_state_structure; use dpp::block::block_info::BlockInfo; use dpp::data_contract::accessors::v0::{DataContractV0Getters, DataContractV0Setters}; @@ -154,7 +155,6 @@ mod tests { use dpp::data_contract::DataContract; use dpp::tests::json_document::json_document_to_contract; use dpp::version::PlatformVersion; - use grovedb_epoch_based_storage_flags::StorageFlags; const PLAIN_CONTRACT: &str = "tests/supporting_files/contract/family/family-contract.json"; const HISTORY_CONTRACT: &str = From e183c6e2c3b39d4093dbc6228ab6b2d78440d395 Mon Sep 17 00:00:00 2001 From: DCG-Claude Date: Wed, 16 Sep 2026 07:35:23 -0500 Subject: [PATCH 03/13] feat(drive): carry recorded refund owners on batch apply costs Adds LowLevelDriveOperation::CalculatedCostOperationWithRefundOwners, a measured cost whose sectioned storage removal comes with the typed owner recorded for every carrier key. operation_cost rejects it so the shipped fee decoder fails closed on it through its catch-all arm; combine_cost_operations sums it. push_drive_operation_result_with_refund_owners pushes it. grove_apply_batch_with_add_costs and grove_apply_partial_batch_with_add_costs gain a version 1 that splits removed bytes with the typed storage flags and pushes the typed cost operation. No version table selects them yet: the dispatcher tests select version 1 through a test-built drive version and pin that an identity-owned delete yields the same root hash and removed bytes under both generations, that a bucket-owned delete records its owner under version 1, and that the full batch version 0 rejects bucket flags. The shipped partial batch generation inlines its closures and therefore sections bucket bytes without recording an owner; the test pins that too, with the reasoning for why it is acceptable (no production caller, bucket bytes exist only from the version that selects the typed generation). Refs #4689 Co-Authored-By: Claude Fable 5.1 --- packages/rs-drive/src/fees/op.rs | 129 ++++++++++- .../grove_apply_batch_with_add_costs/mod.rs | 18 +- .../grove_apply_batch_with_add_costs/tests.rs | 205 ++++++++++++++++++ .../v1/mod.rs | 123 +++++++++++ .../mod.rs | 19 +- .../tests.rs | 161 ++++++++++++++ .../v1/mod.rs | 85 ++++++++ .../rs-drive/src/util/grove_operations/mod.rs | 27 ++- 8 files changed, 756 insertions(+), 11 deletions(-) create mode 100644 packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/tests.rs create mode 100644 packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/v1/mod.rs create mode 100644 packages/rs-drive/src/util/grove_operations/grove_apply_partial_batch_with_add_costs/tests.rs create mode 100644 packages/rs-drive/src/util/grove_operations/grove_apply_partial_batch_with_add_costs/v1/mod.rs diff --git a/packages/rs-drive/src/fees/op.rs b/packages/rs-drive/src/fees/op.rs index e8434cdc4f7..e701689ad51 100644 --- a/packages/rs-drive/src/fees/op.rs +++ b/packages/rs-drive/src/fees/op.rs @@ -20,8 +20,9 @@ use crate::error::fee::FeeError; use crate::error::Error; use crate::fees::get_overflow_error; use crate::fees::op::LowLevelDriveOperation::{ - CalculatedCostOperation, CalculatedEphemeralCostOperation, EphemeralGroveOperation, - FunctionOperation, GroveOperation, PreCalculatedFeeResult, RepaidIdentityDebt, + CalculatedCostOperation, CalculatedCostOperationWithRefundOwners, + CalculatedEphemeralCostOperation, EphemeralGroveOperation, FunctionOperation, GroveOperation, + PreCalculatedFeeResult, RepaidIdentityDebt, }; use crate::util::batch::grovedb_op_batch::GroveDbOpBatchV0Methods; #[cfg(test)] @@ -29,7 +30,7 @@ use crate::util::grove_operations::pending_grove_operations::count_copied_pendin use crate::util::storage_flags::StorageFlags; use dpp::block::epoch::Epoch; use dpp::fee::default_costs::CachedEpochIndexFeeVersions; -use dpp::fee::fee_result::refunds::FeeRefunds; +use dpp::fee::fee_result::refunds::{FeeRefunds, RefundOwnersByIdentifier}; use dpp::fee::fee_result::{FeeResult, LifetimeStorageFees}; use dpp::fee::Credits; use platform_version::version::fee::FeeVersion; @@ -238,6 +239,22 @@ pub enum LowLevelDriveOperation { /// `add_to_identity_balance_operations` 1; an apply that meets one it does not route fails /// instead of dropping it. RepaidIdentityDebt(Credits), + /// A calculated cost whose sectioned storage removal carries the typed + /// owner recorded for every carrier key when the bytes were split. + /// + /// Pushed by the batch apply generations that split removed bytes with + /// typed storage flags. Only a fee decoder that knows how to route typed + /// owners may consume it: `operation_cost` rejects it, so a decoder that + /// predates typed owners fails closed instead of pricing a removal whose + /// owner it cannot name. + CalculatedCostOperationWithRefundOwners { + /// The measured cost, whose removed bytes are sectioned under the + /// owners' carrier keys + cost: OperationCost, + /// The recorded owner of every carrier key in the sectioned removal, + /// the system key excepted + refund_owners: RefundOwnersByIdentifier, + }, } /// How the added bytes of an ephemeral batch are priced. @@ -476,6 +493,12 @@ impl LowLevelDriveOperation { RepaidIdentityDebt(_) => Err(Error::Drive(DriveError::CorruptedCodeExecution( "a repaid identity debt must be routed to the processing fee pool, not priced", ))), + CalculatedCostOperationWithRefundOwners { .. } => { + Err(Error::Drive(DriveError::CorruptedCodeExecution( + "a cost operation carrying typed refund owners reached a fee decoder that \ + cannot route typed owners", + ))) + } } } @@ -513,10 +536,13 @@ impl LowLevelDriveOperation { /// Filters the groveDB ops from a list of operations and puts them in a `GroveDbOpBatch`. pub fn combine_cost_operations(operations: &[LowLevelDriveOperation]) -> OperationCost { let mut cost = OperationCost::default(); - operations.iter().for_each(|op| { - if let CalculatedCostOperation(operation_cost) = op { - cost += operation_cost.clone() - } + operations.iter().for_each(|op| match op { + CalculatedCostOperation(operation_cost) + | CalculatedCostOperationWithRefundOwners { + cost: operation_cost, + .. + } => cost += operation_cost.clone(), + _ => {} }); cost } @@ -1964,8 +1990,13 @@ impl DriveCost for OperationCost { #[allow(clippy::identity_op)] mod tests { use super::*; - use grovedb_costs::storage_cost::removal::StorageRemovedBytes; + use dpp::fee::refund_owner::RefundOwner; + use dpp::identifier::Identifier; + use grovedb_costs::storage_cost::removal::{ + StorageRemovalPerEpochByIdentifier, StorageRemovedBytes, + }; use grovedb_costs::storage_cost::StorageCost; + use intmap::IntMap; use platform_version::version::fee::storage::FeeStorageVersion; use platform_version::version::fee::FeeVersion; @@ -2339,6 +2370,88 @@ mod tests { ); } + #[test] + fn should_reject_a_typed_cost_operation_from_operation_cost() { + let op = CalculatedCostOperationWithRefundOwners { + cost: OperationCost::default(), + refund_owners: Default::default(), + }; + let result = op.operation_cost(); + let err_msg = format!("{:?}", result.expect_err("typed costs are not plain costs")); + assert!( + err_msg.contains("cannot route typed owners"), + "unexpected error: {}", + err_msg + ); + } + + /// The shipped decoder reaches `operation_cost` through its catch-all + /// arm, so a typed cost operation makes it fail closed instead of pricing + /// a removal whose owner it cannot route. + #[test] + fn should_fail_closed_when_consume_to_fees_v0_meets_a_typed_cost_operation() { + let owner = RefundOwner::Identity(Identifier::from([5u8; 32])); + let mut removal = StorageRemovalPerEpochByIdentifier::new(); + removal.insert(owner.removal_key(), IntMap::from_iter([(0u16, 100u32)])); + let op = CalculatedCostOperationWithRefundOwners { + cost: OperationCost { + seek_count: 1, + storage_cost: StorageCost { + added_bytes: 0, + replaced_bytes: 0, + removed_bytes: SectionedStorageRemoval(removal), + }, + storage_loaded_bytes: 0, + hash_node_calls: 0, + sinsemilla_hash_calls: 0, + }, + refund_owners: BTreeMap::from([(owner.removal_key(), owner)]), + }; + + let result = LowLevelDriveOperation::consume_to_fees_v0( + vec![op], + &Epoch::new(1).expect("epoch"), + 20, + fee_version(), + None, + ); + + assert!(matches!( + result, + Err(Error::Drive(DriveError::CorruptedCodeExecution(_))) + )); + } + + #[test] + fn should_sum_typed_cost_operations_into_combined_costs() { + let cost = OperationCost { + seek_count: 4, + storage_cost: StorageCost { + added_bytes: 7, + replaced_bytes: 3, + removed_bytes: StorageRemovedBytes::NoStorageRemoval, + }, + storage_loaded_bytes: 11, + hash_node_calls: 2, + sinsemilla_hash_calls: 0, + }; + let operations = vec![ + CalculatedCostOperation(cost.clone()), + CalculatedCostOperationWithRefundOwners { + cost: cost.clone(), + refund_owners: Default::default(), + }, + ]; + + let combined = LowLevelDriveOperation::combine_cost_operations(&operations); + + assert_eq!(combined.seek_count, 8); + assert_eq!(combined.storage_cost.added_bytes, 14); + assert_eq!(combined.storage_cost.replaced_bytes, 6); + assert_eq!(combined.storage_loaded_bytes, 22); + assert_eq!(combined.hash_node_calls, 4); + } + // --------------------------------------------------------------- // 6. combine_cost_operations — filter and sum // --------------------------------------------------------------- diff --git a/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/mod.rs b/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/mod.rs index ccea6a9242a..31c316d7af5 100644 --- a/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/mod.rs +++ b/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/mod.rs @@ -1,4 +1,8 @@ mod v0; +mod v1; + +#[cfg(test)] +mod tests; use crate::util::batch::GroveDbOpBatch; @@ -14,6 +18,11 @@ use grovedb::TransactionArg; impl Drive { /// Applies the given groveDB operations batch and gets and passes the costs to `push_drive_operation_result`. /// + /// Version 0 splits removed bytes with the identity-only storage flags + /// and pushes a plain cost operation. Version 1 splits with the typed + /// storage flags and pushes a cost operation that carries the recorded + /// refund owner of every sectioned removal. + /// /// # Parameters /// * `ops`: The groveDB operations batch. /// * `validate`: Specifies whether to validate that insertions do not override existing entries. @@ -40,9 +49,16 @@ impl Drive { drive_operations, drive_version, ), + 1 => self.grove_apply_batch_with_add_costs_v1( + ops, + validate, + transaction, + drive_operations, + drive_version, + ), version => Err(Error::Drive(DriveError::UnknownVersionMismatch { method: "grove_apply_batch_with_add_costs".to_string(), - known_versions: vec![0], + known_versions: vec![0, 1], received: version, })), } diff --git a/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/tests.rs b/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/tests.rs new file mode 100644 index 00000000000..9f23f61f4cd --- /dev/null +++ b/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/tests.rs @@ -0,0 +1,205 @@ +//! Tests that run both batch apply generations through the dispatcher. +//! +//! Version 1 is selected only by a test-built drive version: no protocol +//! version references it until the fee decoder that consumes the typed cost +//! operation lands. + +use crate::drive::system::misc_path_vec; +use crate::drive::Drive; +use crate::error::Error; +use crate::fees::op::LowLevelDriveOperation; +use crate::util::batch::grovedb_op_batch::GroveDbOpBatchV0Methods; +use crate::util::batch::GroveDbOpBatch; +use crate::util::storage_flags::StorageFlags; +use crate::util::test_helpers::setup::setup_drive_with_initial_state_structure; +use dpp::fee::refund_owner::{RefundOwner, RefundOwnersByIdentifier}; +use dpp::identifier::Identifier; +use dpp::version::PlatformVersion; +use grovedb::Element; +use grovedb_costs::storage_cost::removal::StorageRemovedBytes; +use grovedb_costs::storage_cost::removal::StorageRemovedBytes::SectionedStorageRemoval; +use grovedb_costs::OperationCost; +use platform_version::version::drive_versions::DriveVersion; + +const OWNER_ID: [u8; 32] = [0x22; 32]; +const CONTRACT_ID: [u8; 32] = [0x11; 32]; +const KEY: &[u8] = b"fix-08-flagged-item"; + +/// The latest drive version with both batch apply slots moved to version 1. +fn typed_drive_version() -> DriveVersion { + let mut drive_version = PlatformVersion::latest().drive.clone(); + drive_version.grove_methods.apply.grove_apply_batch = 1; + drive_version.grove_methods.apply.grove_apply_partial_batch = 1; + drive_version +} + +/// Inserts a flagged 200 byte item under the misc tree with the shipped +/// generation, which accepts the historical flag types. +fn insert_flagged_item(drive: &Drive, flags: &StorageFlags, drive_version: &DriveVersion) { + let mut batch = GroveDbOpBatch::new(); + batch.add_insert( + misc_path_vec(), + KEY.to_vec(), + Element::new_item_with_flags(vec![7u8; 200], flags.to_some_element_flags()), + ); + drive + .grove_apply_batch_with_add_costs(batch, false, None, &mut vec![], drive_version) + .expect("should insert the flagged item"); +} + +fn delete_item( + drive: &Drive, + drive_version: &DriveVersion, +) -> Result, Error> { + let mut batch = GroveDbOpBatch::new(); + batch.add_delete(misc_path_vec(), KEY.to_vec()); + let mut drive_operations = vec![]; + drive.grove_apply_batch_with_add_costs( + batch, + false, + None, + &mut drive_operations, + drive_version, + )?; + Ok(drive_operations) +} + +fn root_hash(drive: &Drive, drive_version: &DriveVersion) -> [u8; 32] { + drive + .grove + .root_hash(None, &drive_version.grove_version) + .unwrap() + .expect("should get root hash") +} + +fn sectioned_removal(cost: &OperationCost) -> &StorageRemovedBytes { + &cost.storage_cost.removed_bytes +} + +#[test] +fn should_delete_an_identity_owned_item_identically_under_both_generations() { + let shipped = PlatformVersion::latest().drive.clone(); + let typed = typed_drive_version(); + let flags = StorageFlags::SingleEpochOwned(3, OWNER_ID); + + let drive_v0 = setup_drive_with_initial_state_structure(None); + insert_flagged_item(&drive_v0, &flags, &shipped); + let drive_v1 = setup_drive_with_initial_state_structure(None); + insert_flagged_item(&drive_v1, &flags, &shipped); + assert_eq!( + root_hash(&drive_v0, &shipped), + root_hash(&drive_v1, &shipped) + ); + + let ops_v0 = delete_item(&drive_v0, &shipped).expect("v0 should delete"); + let ops_v1 = delete_item(&drive_v1, &typed).expect("v1 should delete"); + + assert_eq!(root_hash(&drive_v0, &shipped), root_hash(&drive_v1, &typed)); + + let [LowLevelDriveOperation::CalculatedCostOperation(cost_v0)] = ops_v0.as_slice() else { + panic!("v0 should push one plain cost operation, got {:?}", ops_v0); + }; + let [LowLevelDriveOperation::CalculatedCostOperationWithRefundOwners { + cost: cost_v1, + refund_owners, + }] = ops_v1.as_slice() + else { + panic!("v1 should push one typed cost operation, got {:?}", ops_v1); + }; + + assert_eq!(cost_v0, cost_v1, "only the operation shape differs"); + let SectionedStorageRemoval(removal) = sectioned_removal(cost_v1) else { + panic!("an owned delete sections its removed bytes"); + }; + assert_eq!(removal.keys().copied().collect::>(), vec![OWNER_ID]); + assert_eq!( + *refund_owners, + RefundOwnersByIdentifier::from([( + OWNER_ID, + RefundOwner::Identity(Identifier::from(OWNER_ID)) + )]) + ); +} + +#[test] +fn should_record_the_bucket_owner_when_a_bucket_owned_item_is_deleted_under_v1() { + let typed = typed_drive_version(); + let owner = RefundOwner::ContractBucket { + contract_id: Identifier::from(CONTRACT_ID), + position: 7, + }; + let flags = StorageFlags::new_single_epoch_for_owner(3, Some(owner)); + + let drive = setup_drive_with_initial_state_structure(None); + // the insert carries the flags through unchanged: no closure parses + // flags on a plain insert + insert_flagged_item(&drive, &flags, &typed); + + let ops = delete_item(&drive, &typed).expect("v1 should delete a bucket owned item"); + + let [LowLevelDriveOperation::CalculatedCostOperationWithRefundOwners { + cost, + refund_owners, + }] = ops.as_slice() + else { + panic!("v1 should push one typed cost operation, got {:?}", ops); + }; + let SectionedStorageRemoval(removal) = sectioned_removal(cost) else { + panic!("an owned delete sections its removed bytes"); + }; + assert_eq!( + removal.keys().copied().collect::>(), + vec![owner.removal_key()] + ); + let bytes_removed: u32 = removal[&owner.removal_key()].values().sum(); + assert!(bytes_removed >= 200, "the item value is at least 200 bytes"); + assert_eq!( + *refund_owners, + RefundOwnersByIdentifier::from([(owner.removal_key(), owner)]) + ); +} + +#[test] +fn should_fail_closed_when_a_bucket_owned_item_is_deleted_under_v0() { + let shipped = PlatformVersion::latest().drive.clone(); + let typed = typed_drive_version(); + let owner = RefundOwner::ContractBucket { + contract_id: Identifier::from(CONTRACT_ID), + position: 7, + }; + let flags = StorageFlags::new_single_epoch_for_owner(3, Some(owner)); + + let drive = setup_drive_with_initial_state_structure(None); + insert_flagged_item(&drive, &flags, &typed); + let before = root_hash(&drive, &shipped); + + let error = delete_item(&drive, &shipped).expect_err("v0 cannot attribute bucket owned bytes"); + + assert!( + error + .to_string() + .contains("unknown storage flags serialization"), + "unexpected error: {}", + error + ); + assert_eq!(root_hash(&drive, &shipped), before, "nothing was applied"); +} + +#[test] +fn should_reject_an_unknown_generation() { + let drive = setup_drive_with_initial_state_structure(None); + let mut drive_version = PlatformVersion::latest().drive.clone(); + drive_version.grove_methods.apply.grove_apply_batch = 2; + + let mut batch = GroveDbOpBatch::new(); + batch.add_delete(misc_path_vec(), KEY.to_vec()); + + let error = drive + .grove_apply_batch_with_add_costs(batch, false, None, &mut vec![], &drive_version) + .expect_err("version 2 does not exist"); + + assert!(matches!( + error, + Error::Drive(crate::error::drive::DriveError::UnknownVersionMismatch { .. }) + )); +} diff --git a/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/v1/mod.rs b/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/v1/mod.rs new file mode 100644 index 00000000000..0c7918b579a --- /dev/null +++ b/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/v1/mod.rs @@ -0,0 +1,123 @@ +use crate::drive::Drive; +use crate::error::drive::DriveError; +use crate::error::Error; +use crate::fees::op::LowLevelDriveOperation; +use crate::query::GroveError; +use crate::util::batch::grovedb_op_batch::GroveDbOpBatchV0Methods; +use crate::util::batch::GroveDbOpBatch; +use crate::util::grove_operations::push_drive_operation_result_with_refund_owners; +use crate::util::storage_flags::StorageFlags; +use dpp::fee::refund_owner::RefundOwnersByIdentifier; +use grovedb::batch::{BatchApplyOptions, QualifiedGroveDbOp}; +use grovedb::TransactionArg; +use platform_version::version::drive_versions::DriveVersion; + +impl Drive { + /// Applies the given groveDB operations batch and passes the costs to + /// `push_drive_operation_result_with_refund_owners`. + /// + /// This generation splits removed bytes with the typed storage flags: + /// every owned removal is sectioned under the owner's carrier key and + /// the owner is recorded next to it, so the cost operation it pushes + /// carries the recorded owners for the fee decoder to route. It accepts + /// contract bucket owned flags, which the previous generation rejects. + pub(super) fn grove_apply_batch_with_add_costs_v1( + &self, + ops: GroveDbOpBatch, + validate: bool, + transaction: TransactionArg, + drive_operations: &mut Vec, + drive_version: &DriveVersion, + ) -> Result<(), Error> { + if ops.is_empty() { + return Err(Error::Drive(DriveError::BatchIsEmpty( + "batch is empty when trying to apply batch with add costs".to_string(), + ))); + } + + if self.config.batching_consistency_verification { + let consistency_results = + QualifiedGroveDbOp::verify_consistency_of_operations(&ops.operations); + if !consistency_results.is_empty() { + tracing::error!( + ?consistency_results, + "grovedb consistency verification failed" + ); + return Err(Error::Drive(DriveError::GroveDBInsertion( + "insertion order error", + ))); + } + } + + // Clone ops only if we log them + #[cfg(feature = "grovedb_operations_logging")] + let maybe_params_for_logs = if tracing::event_enabled!(target: "drive_grovedb_operations", tracing::Level::TRACE) + { + let root_hash = self + .grove + .root_hash(transaction, &drive_version.grove_version) + .unwrap() + .map_err(Error::from)?; + + Some((ops.clone(), root_hash)) + } else { + None + }; + + let mut refund_owners = RefundOwnersByIdentifier::new(); + + let cost_context = self.grove.apply_batch_with_element_flags_update( + ops.operations, + Some(BatchApplyOptions { + validate_insertion_does_not_override: validate, + validate_insertion_does_not_override_tree: validate, + disable_operation_consistency_check: !self.config.batching_consistency_verification, + base_root_storage_is_free: true, + batch_pause_height: None, + }), + |cost, old_flags, new_flags| { + StorageFlags::update_element_flags_typed(cost, old_flags, new_flags) + .map_err(|e| GroveError::JustInTimeElementFlagsClientError(e.to_string())) + }, + |flags, removed_key_bytes, removed_value_bytes| { + StorageFlags::split_removal_bytes_typed( + flags, + removed_key_bytes, + removed_value_bytes, + &mut refund_owners, + ) + .map_err(|e| GroveError::SplitRemovalBytesClientError(e.to_string())) + }, + transaction, + &drive_version.grove_version, + ); + + #[cfg(feature = "grovedb_operations_logging")] + if tracing::event_enabled!(target: "drive_grovedb_operations", tracing::Level::TRACE) + && cost_context.value.is_ok() + { + if let Some((ops, previous_root_hash)) = maybe_params_for_logs { + let root_hash = self + .grove + .root_hash(transaction, &drive_version.grove_version) + .unwrap() + .map_err(Error::from)?; + + tracing::trace!( + target: "drive_grovedb_operations", + ?ops, + ?root_hash, + ?previous_root_hash, + is_transactional = transaction.is_some(), + "grovedb batch applied", + ); + } + } + + push_drive_operation_result_with_refund_owners( + cost_context, + refund_owners, + drive_operations, + ) + } +} diff --git a/packages/rs-drive/src/util/grove_operations/grove_apply_partial_batch_with_add_costs/mod.rs b/packages/rs-drive/src/util/grove_operations/grove_apply_partial_batch_with_add_costs/mod.rs index 7ba94976f5b..5c6ebb1a430 100644 --- a/packages/rs-drive/src/util/grove_operations/grove_apply_partial_batch_with_add_costs/mod.rs +++ b/packages/rs-drive/src/util/grove_operations/grove_apply_partial_batch_with_add_costs/mod.rs @@ -1,4 +1,8 @@ mod v0; +mod v1; + +#[cfg(test)] +mod tests; use crate::util::batch::GroveDbOpBatch; @@ -16,6 +20,11 @@ use grovedb_costs::OperationCost; impl Drive { /// Applies the given groveDB operations batch, gets and passes the costs to `push_drive_operation_result`. /// + /// Version 0 splits removed bytes with the identity-only storage flags + /// and pushes a plain cost operation. Version 1 splits with the typed + /// storage flags and pushes a cost operation that carries the recorded + /// refund owner of every sectioned removal. + /// /// # Parameters /// * `ops`: The batch of groveDB operations to retrieve costs for. /// * `validate`: Specifies whether to validate that insertions do not override existing entries. @@ -49,9 +58,17 @@ impl Drive { drive_operations, drive_version, ), + 1 => self.grove_apply_partial_batch_with_add_costs_v1( + ops, + validate, + transaction, + add_on_operations, + drive_operations, + drive_version, + ), version => Err(Error::Drive(DriveError::UnknownVersionMismatch { method: "grove_apply_partial_batch_with_add_costs".to_string(), - known_versions: vec![0], + known_versions: vec![0, 1], received: version, })), } diff --git a/packages/rs-drive/src/util/grove_operations/grove_apply_partial_batch_with_add_costs/tests.rs b/packages/rs-drive/src/util/grove_operations/grove_apply_partial_batch_with_add_costs/tests.rs new file mode 100644 index 00000000000..ad5438fb840 --- /dev/null +++ b/packages/rs-drive/src/util/grove_operations/grove_apply_partial_batch_with_add_costs/tests.rs @@ -0,0 +1,161 @@ +//! Tests that run both partial batch apply generations through the +//! dispatcher. Version 1 is selected only by a test-built drive version. + +use crate::drive::system::misc_path_vec; +use crate::drive::Drive; +use crate::error::Error; +use crate::fees::op::LowLevelDriveOperation; +use crate::util::batch::grovedb_op_batch::GroveDbOpBatchV0Methods; +use crate::util::batch::GroveDbOpBatch; +use crate::util::storage_flags::StorageFlags; +use crate::util::test_helpers::setup::setup_drive_with_initial_state_structure; +use dpp::fee::refund_owner::{RefundOwner, RefundOwnersByIdentifier}; +use dpp::identifier::Identifier; +use dpp::version::PlatformVersion; +use grovedb::Element; +use grovedb_costs::storage_cost::removal::StorageRemovedBytes::SectionedStorageRemoval; +use platform_version::version::drive_versions::DriveVersion; + +const OWNER_ID: [u8; 32] = [0x33; 32]; +const CONTRACT_ID: [u8; 32] = [0x44; 32]; +const KEY: &[u8] = b"fix-08-partial-flagged-item"; + +fn typed_drive_version() -> DriveVersion { + let mut drive_version = PlatformVersion::latest().drive.clone(); + drive_version.grove_methods.apply.grove_apply_batch = 1; + drive_version.grove_methods.apply.grove_apply_partial_batch = 1; + drive_version +} + +fn insert_flagged_item(drive: &Drive, flags: &StorageFlags, drive_version: &DriveVersion) { + let mut batch = GroveDbOpBatch::new(); + batch.add_insert( + misc_path_vec(), + KEY.to_vec(), + Element::new_item_with_flags(vec![9u8; 150], flags.to_some_element_flags()), + ); + drive + .grove_apply_batch_with_add_costs(batch, false, None, &mut vec![], drive_version) + .expect("should insert the flagged item"); +} + +fn delete_item_partially( + drive: &Drive, + drive_version: &DriveVersion, +) -> Result, Error> { + let mut batch = GroveDbOpBatch::new(); + batch.add_delete(misc_path_vec(), KEY.to_vec()); + let mut drive_operations = vec![]; + drive.grove_apply_partial_batch_with_add_costs( + batch, + false, + None, + |_cost, _ops_by_level| Ok(vec![]), + &mut drive_operations, + drive_version, + )?; + Ok(drive_operations) +} + +#[test] +fn should_push_the_same_cost_for_an_identity_owned_delete_under_both_generations() { + let shipped = PlatformVersion::latest().drive.clone(); + let typed = typed_drive_version(); + let flags = StorageFlags::SingleEpochOwned(2, OWNER_ID); + + let drive_v0 = setup_drive_with_initial_state_structure(None); + insert_flagged_item(&drive_v0, &flags, &shipped); + let drive_v1 = setup_drive_with_initial_state_structure(None); + insert_flagged_item(&drive_v1, &flags, &shipped); + + let ops_v0 = delete_item_partially(&drive_v0, &shipped).expect("v0 should delete"); + let ops_v1 = delete_item_partially(&drive_v1, &typed).expect("v1 should delete"); + + let [LowLevelDriveOperation::CalculatedCostOperation(cost_v0)] = ops_v0.as_slice() else { + panic!("v0 should push one plain cost operation, got {:?}", ops_v0); + }; + let [LowLevelDriveOperation::CalculatedCostOperationWithRefundOwners { + cost: cost_v1, + refund_owners, + }] = ops_v1.as_slice() + else { + panic!("v1 should push one typed cost operation, got {:?}", ops_v1); + }; + assert_eq!(cost_v0, cost_v1); + assert_eq!( + *refund_owners, + RefundOwnersByIdentifier::from([( + OWNER_ID, + RefundOwner::Identity(Identifier::from(OWNER_ID)) + )]) + ); +} + +#[test] +fn should_record_the_bucket_owner_when_a_bucket_owned_item_is_deleted_under_v1() { + let typed = typed_drive_version(); + let owner = RefundOwner::ContractBucket { + contract_id: Identifier::from(CONTRACT_ID), + position: 1, + }; + let flags = StorageFlags::new_single_epoch_for_owner(2, Some(owner)); + + let drive = setup_drive_with_initial_state_structure(None); + insert_flagged_item(&drive, &flags, &typed); + + let ops = delete_item_partially(&drive, &typed).expect("v1 should delete"); + let [LowLevelDriveOperation::CalculatedCostOperationWithRefundOwners { + cost, + refund_owners, + }] = ops.as_slice() + else { + panic!("v1 should push one typed cost operation, got {:?}", ops); + }; + let SectionedStorageRemoval(removal) = &cost.storage_cost.removed_bytes else { + panic!("an owned delete sections its removed bytes"); + }; + assert_eq!( + removal.keys().copied().collect::>(), + vec![owner.removal_key()] + ); + assert_eq!( + *refund_owners, + RefundOwnersByIdentifier::from([(owner.removal_key(), owner)]) + ); +} + +/// The shipped partial batch generation inlines its flag closures instead of +/// calling the crate-delegating entry points, so it parses bucket flags +/// through the shared readers and sections the bytes under the carrier key +/// without recording the owner. Unlike the full batch generation it does not +/// fail closed. This is acceptable because no production path applies a +/// partial batch (only its own module and the version tables name it) and +/// bucket owned bytes are written only from the protocol version whose +/// tables select version 1 of both batch apply methods. This test pins that +/// reasoning: if a caller of the partial batch ever appears, the assertion +/// below is the place that has to change. +#[test] +fn should_section_bucket_bytes_without_a_recorded_owner_under_the_shipped_partial_generation() { + let shipped = PlatformVersion::latest().drive.clone(); + let typed = typed_drive_version(); + let owner = RefundOwner::ContractBucket { + contract_id: Identifier::from(CONTRACT_ID), + position: 1, + }; + let flags = StorageFlags::new_single_epoch_for_owner(2, Some(owner)); + + let drive = setup_drive_with_initial_state_structure(None); + insert_flagged_item(&drive, &flags, &typed); + + let ops = delete_item_partially(&drive, &shipped).expect("v0 partial applies the delete"); + let [LowLevelDriveOperation::CalculatedCostOperation(cost)] = ops.as_slice() else { + panic!("v0 pushes one plain cost operation, got {:?}", ops); + }; + let SectionedStorageRemoval(removal) = &cost.storage_cost.removed_bytes else { + panic!("an owned delete sections its removed bytes"); + }; + assert_eq!( + removal.keys().copied().collect::>(), + vec![owner.removal_key()] + ); +} diff --git a/packages/rs-drive/src/util/grove_operations/grove_apply_partial_batch_with_add_costs/v1/mod.rs b/packages/rs-drive/src/util/grove_operations/grove_apply_partial_batch_with_add_costs/v1/mod.rs new file mode 100644 index 00000000000..a181e93fc6d --- /dev/null +++ b/packages/rs-drive/src/util/grove_operations/grove_apply_partial_batch_with_add_costs/v1/mod.rs @@ -0,0 +1,85 @@ +use crate::drive::Drive; +use crate::error::drive::DriveError; +use crate::error::Error; +use crate::fees::op::LowLevelDriveOperation; +use crate::query::GroveError; +use crate::util::batch::grovedb_op_batch::GroveDbOpBatchV0Methods; +use crate::util::batch::GroveDbOpBatch; +use crate::util::grove_operations::push_drive_operation_result_with_refund_owners; +use crate::util::storage_flags::StorageFlags; +use dpp::fee::refund_owner::RefundOwnersByIdentifier; +use grovedb::batch::{BatchApplyOptions, OpsByLevelPath, QualifiedGroveDbOp}; +use grovedb::TransactionArg; +use grovedb_costs::OperationCost; +use platform_version::version::drive_versions::DriveVersion; + +impl Drive { + /// Applies the given groveDB operations batch with add-on operations and + /// passes the costs to `push_drive_operation_result_with_refund_owners`. + /// + /// This generation splits removed bytes with the typed storage flags and + /// records the owner of every owned removal on the cost operation it + /// pushes. The flag update closure is the typed one as well, which also + /// removes the previous generation's inline copy of that logic. + pub(super) fn grove_apply_partial_batch_with_add_costs_v1( + &self, + ops: GroveDbOpBatch, + validate: bool, + transaction: TransactionArg, + add_on_operations: impl FnMut( + &OperationCost, + &Option, + ) -> Result, GroveError>, + drive_operations: &mut Vec, + drive_version: &DriveVersion, + ) -> Result<(), Error> { + if ops.is_empty() { + return Err(Error::Drive(DriveError::BatchIsEmpty( + "batch is empty when trying to apply partial batch with add costs".to_string(), + ))); + } + if self.config.batching_consistency_verification { + let consistency_results = + QualifiedGroveDbOp::verify_consistency_of_operations(&ops.operations); + if !consistency_results.is_empty() { + return Err(Error::Drive(DriveError::GroveDBInsertion( + "insertion order error", + ))); + } + } + + let mut refund_owners = RefundOwnersByIdentifier::new(); + + let cost_context = self.grove.apply_partial_batch_with_element_flags_update( + ops.operations, + Some(BatchApplyOptions { + validate_insertion_does_not_override: validate, + validate_insertion_does_not_override_tree: validate, + disable_operation_consistency_check: false, + base_root_storage_is_free: true, + batch_pause_height: None, + }), + |cost, old_flags, new_flags| { + StorageFlags::update_element_flags_typed(cost, old_flags, new_flags) + .map_err(|e| GroveError::JustInTimeElementFlagsClientError(e.to_string())) + }, + |flags, removed_key_bytes, removed_value_bytes| { + StorageFlags::split_removal_bytes_typed( + flags, + removed_key_bytes, + removed_value_bytes, + &mut refund_owners, + ) + .map_err(|e| GroveError::SplitRemovalBytesClientError(e.to_string())) + }, + add_on_operations, + transaction, + &drive_version.grove_version, + ); + push_drive_operation_result_with_refund_owners( + cost_context, + refund_owners, + drive_operations, + ) + } +} diff --git a/packages/rs-drive/src/util/grove_operations/mod.rs b/packages/rs-drive/src/util/grove_operations/mod.rs index 93c959791cd..46ba4e1bc14 100644 --- a/packages/rs-drive/src/util/grove_operations/mod.rs +++ b/packages/rs-drive/src/util/grove_operations/mod.rs @@ -221,7 +221,10 @@ use grovedb::{EstimatedLayerInformation, MaybeTree, TreeType}; use crate::error::Error; use crate::fees::op::LowLevelDriveOperation; -use crate::fees::op::LowLevelDriveOperation::CalculatedCostOperation; +use crate::fees::op::LowLevelDriveOperation::{ + CalculatedCostOperation, CalculatedCostOperationWithRefundOwners, +}; +use dpp::fee::fee_result::refunds::RefundOwnersByIdentifier; use grovedb::Error as GroveError; @@ -240,6 +243,28 @@ pub(crate) fn push_drive_operation_result( value.map_err(Error::from) } +/// Pushes an operation's `OperationCost` together with the refund owners +/// recorded while its removed bytes were split, and returns the operation's +/// return value. +/// +/// The owners ride with the cost as `CalculatedCostOperationWithRefundOwners` +/// so that the fee decoder reads each carrier key's owner from the record +/// rather than inferring it from the key. +fn push_drive_operation_result_with_refund_owners( + cost_context: CostContext>, + refund_owners: RefundOwnersByIdentifier, + drive_operations: &mut Vec, +) -> Result { + let CostContext { value, cost } = cost_context; + if !cost.is_nothing() { + drive_operations.push(CalculatedCostOperationWithRefundOwners { + cost, + refund_owners, + }); + } + value.map_err(Error::from) +} + /// Pushes an operation's `OperationCost` to `drive_operations` given its `CostContext` /// if `drive_operations` is given. Returns the operation's return value. fn push_drive_operation_result_optional( From 85feb2360754421f4f8c2d0569a413e42fb15c48 Mon Sep 17 00:00:00 2001 From: DCG-Claude Date: Wed, 16 Sep 2026 07:35:25 -0500 Subject: [PATCH 04/13] docs: describe typed refund owners and the typed cost operation in the book Refs #4689 Co-Authored-By: Claude Fable 5.1 --- book/src/drive/cost-tracking.md | 12 ++++++++++-- book/src/fees/overview.md | 26 +++++++++++++++++++++++--- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/book/src/drive/cost-tracking.md b/book/src/drive/cost-tracking.md index b24e8e67737..094c2d206a5 100644 --- a/book/src/drive/cost-tracking.md +++ b/book/src/drive/cost-tracking.md @@ -22,10 +22,14 @@ pub enum LowLevelDriveOperation { FunctionOperation(FunctionOp), CalculatedCostOperation(OperationCost), PreCalculatedFeeResult(FeeResult), + CalculatedCostOperationWithRefundOwners { + cost: OperationCost, + refund_owners: RefundOwnersByIdentifier, + }, } ``` -Four variants, each representing a different kind of cost: +Five variants, each representing a different kind of cost: ### GroveOperation @@ -108,6 +112,10 @@ impl FunctionOp { A fee result that was already computed elsewhere and just needs to be included in the total. This is a pass-through -- no further calculation needed. +### CalculatedCostOperationWithRefundOwners + +A `CalculatedCostOperation` whose sectioned storage removal comes with the typed `RefundOwner` recorded for every carrier key when the removed bytes were split. It is pushed by the batch apply generations that split removed bytes with typed storage flags (`push_drive_operation_result_with_refund_owners`). `operation_cost()` rejects it on purpose: a fee decoder that predates typed owners reaches `operation_cost()` through its catch-all arm and therefore fails closed instead of pricing a refund whose owner it cannot route. `combine_cost_operations` sums its cost like any other. See [Refunds](../fees/overview.md#refunds) for how the owners are recorded. + ## BaseOp: Arithmetic Operation Costs For simple computational operations (not storage-related), the `BaseOp` enum provides fixed costs: @@ -320,7 +328,7 @@ These provide a cleaner API than constructing `QualifiedGroveDbOp` directly, and - Let operations accumulate in the `drive_operations` vector throughout the call chain. **Do not:** -- Call `operation_cost()` on a `GroveOperation` -- it will return an error. Grove operations must be executed first; only `CalculatedCostOperation` carries a usable cost. +- Call `operation_cost()` on a `GroveOperation` -- it will return an error. Grove operations must be executed first; only `CalculatedCostOperation` carries a usable cost. `CalculatedCostOperationWithRefundOwners` also returns an error there, by design: its owners must be read by a decoder that routes typed owners. - Forget that storage fees and processing fees are calculated differently. Storage fees are proportional to bytes. Processing fees are a complex function of seeks, loads, hashes, and byte movements. - Assume fee rates are constant. They are versioned through `FeeVersion` and can change between protocol versions. - Ignore `removed_bytes_from_system`. This tracks bytes removed that belong to the system rather than a specific identity, affecting the refund calculation. diff --git a/book/src/fees/overview.md b/book/src/fees/overview.md index 94af5b6e964..f36d9d1425e 100644 --- a/book/src/fees/overview.md +++ b/book/src/fees/overview.md @@ -452,13 +452,33 @@ operations and measures the actual cost of each insert, delete, and query. When data is removed from GroveDB (a document is deleted, a key is removed), the system calculates a refund of the original storage fee. Refunds are tracked -per identity per epoch: +per owner per epoch: ```rust -pub struct FeeRefunds(pub CreditsPerEpochByIdentifier); -// BTreeMap> +pub struct FeeRefunds(pub CreditsPerEpochByIdentifier, pub RefundOwnersByIdentifier); +// BTreeMap<[u8; 32], BTreeMap> plus BTreeMap<[u8; 32], RefundOwner> ``` +The first field is the carrier GroveDB hands back: removed bytes keyed by a +32-byte identifier. The second field records the typed owner of every carrier +key. `RefundOwner` (`rs-dpp/src/fee/refund_owner`) names the owner +explicitly: `Identity(id)` for bytes an identity paid for, or +`ContractBucket { contract_id, position }` for bytes a contract credit bucket +paid for. An identity's carrier key is its id, so every historical record +keeps its key; a bucket's carrier key is a domain separated double SHA-256 of +the contract id and the bucket position. The kind is always read from the +record and never inferred from the shape of the key. + +The owner is bound to the bytes when they are stored, in the element flags +(`rs-drive/src/util/storage_flags`). Flag type bytes 0 to 3 are the +historical unowned and identity-owned encodings, produced and parsed by the +pinned `grovedb-epoch-based-storage-flags` crate so they cannot drift. Type +bytes 4 and 5 carry a contract bucket owner. Only the batch apply generation +that knows typed owners splits removed bytes for type bytes 4 and 5 and it +records the owner of every sectioned removal on the cost operation it pushes +(`CalculatedCostOperationWithRefundOwners`); the earlier generation hands the +flags to the crate and fails closed on a type byte it does not know. + Refunds are not 1:1 with the original fee because storage fees are distributed across future epochs (see below). The refund amount depends on how many epochs have elapsed since the data was stored — the longer the data has been stored, the From d2ebd221ba71ecb3de0a3562b3d29db686155b87 Mon Sep 17 00:00:00 2001 From: DCG-Claude Date: Wed, 16 Sep 2026 14:59:58 -0500 Subject: [PATCH 05/13] fix(drive): keep the shipped batch apply generations on the crate flags type Replacing the storage flags re-export with a Drive type silently rebound both shipped batch apply generations through their unchanged import line. The partial batch v0 inlines its closures over the parse helpers, which decode the contract bucket flag types, so over bucket flags it sectioned the bytes under the carrier key and pushed a plain cost operation with no recorded owner; the shipped fee decoder would then have read that key as an identity and credited the wrong account. Both v0 modules now import the crate's StorageFlags directly, the exact type they were compiled against before, so their behaviour is identical to what shipped and they reject type bytes 4 and 5 through the crate's decoder. The Drive type drops the delegating shipped-name closure wrappers, which nothing else called. Dispatcher tests on both methods pin that a bucket-owned delete under generation 0 errors, applies nothing and prices no removal, and that generation 1 then removes it with the recorded owner. Refs #4689 Co-Authored-By: Claude Fable 5.1 --- .../grove_apply_batch_with_add_costs/tests.rs | 27 ++++++- .../v0/mod.rs | 6 +- .../tests.rs | 78 +++++++++++++------ .../v0/mod.rs | 6 +- .../rs-drive/src/util/storage_flags/mod.rs | 18 ++--- .../rs-drive/src/util/storage_flags/split.rs | 16 +--- .../rs-drive/src/util/storage_flags/tests.rs | 16 ++-- .../rs-drive/src/util/storage_flags/update.rs | 14 +--- 8 files changed, 112 insertions(+), 69 deletions(-) diff --git a/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/tests.rs b/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/tests.rs index 9f23f61f4cd..0191570aad2 100644 --- a/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/tests.rs +++ b/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/tests.rs @@ -17,7 +17,9 @@ use dpp::identifier::Identifier; use dpp::version::PlatformVersion; use grovedb::Element; use grovedb_costs::storage_cost::removal::StorageRemovedBytes; -use grovedb_costs::storage_cost::removal::StorageRemovedBytes::SectionedStorageRemoval; +use grovedb_costs::storage_cost::removal::StorageRemovedBytes::{ + NoStorageRemoval, SectionedStorageRemoval, +}; use grovedb_costs::OperationCost; use platform_version::version::drive_versions::DriveVersion; @@ -173,7 +175,12 @@ fn should_fail_closed_when_a_bucket_owned_item_is_deleted_under_v0() { insert_flagged_item(&drive, &flags, &typed); let before = root_hash(&drive, &shipped); - let error = delete_item(&drive, &shipped).expect_err("v0 cannot attribute bucket owned bytes"); + let mut batch = GroveDbOpBatch::new(); + batch.add_delete(misc_path_vec(), KEY.to_vec()); + let mut drive_operations = vec![]; + let error = drive + .grove_apply_batch_with_add_costs(batch, false, None, &mut drive_operations, &shipped) + .expect_err("v0 cannot attribute bucket owned bytes"); assert!( error @@ -183,6 +190,22 @@ fn should_fail_closed_when_a_bucket_owned_item_is_deleted_under_v0() { error ); assert_eq!(root_hash(&drive, &shipped), before, "nothing was applied"); + // the read cost incurred before the failure is still pushed, as for any + // failed grove operation, but no removed bytes and no owners reach the + // fee path + for op in &drive_operations { + let LowLevelDriveOperation::CalculatedCostOperation(cost) = op else { + panic!("only plain read costs may be pushed, got {:?}", op); + }; + assert_eq!(*sectioned_removal(cost), NoStorageRemoval); + } + + // the item is still there and version 1 removes it with the recorded owner + let ops = delete_item(&drive, &typed).expect("v1 should delete"); + assert!(matches!( + ops.as_slice(), + [LowLevelDriveOperation::CalculatedCostOperationWithRefundOwners { .. }] + )); } #[test] diff --git a/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/v0/mod.rs b/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/v0/mod.rs index e44000512d7..d41c3c9b289 100644 --- a/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/v0/mod.rs +++ b/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/v0/mod.rs @@ -6,9 +6,13 @@ use crate::query::GroveError; use crate::util::batch::grovedb_op_batch::GroveDbOpBatchV0Methods; use crate::util::batch::GroveDbOpBatch; use crate::util::grove_operations::push_drive_operation_result; -use crate::util::storage_flags::StorageFlags; +// The shipped generation is bound to the crate's flags type on purpose: it +// splits and combines exactly the four historical flag types and rejects any +// other type byte, so bytes owned by a contract credit bucket can never be +// sectioned by this generation. The typed flags live in version 1. use grovedb::batch::{BatchApplyOptions, QualifiedGroveDbOp}; use grovedb::TransactionArg; +use grovedb_epoch_based_storage_flags::StorageFlags; use platform_version::version::drive_versions::DriveVersion; impl Drive { diff --git a/packages/rs-drive/src/util/grove_operations/grove_apply_partial_batch_with_add_costs/tests.rs b/packages/rs-drive/src/util/grove_operations/grove_apply_partial_batch_with_add_costs/tests.rs index ad5438fb840..2fba1ce3ba6 100644 --- a/packages/rs-drive/src/util/grove_operations/grove_apply_partial_batch_with_add_costs/tests.rs +++ b/packages/rs-drive/src/util/grove_operations/grove_apply_partial_batch_with_add_costs/tests.rs @@ -13,7 +13,9 @@ use dpp::fee::refund_owner::{RefundOwner, RefundOwnersByIdentifier}; use dpp::identifier::Identifier; use dpp::version::PlatformVersion; use grovedb::Element; -use grovedb_costs::storage_cost::removal::StorageRemovedBytes::SectionedStorageRemoval; +use grovedb_costs::storage_cost::removal::StorageRemovedBytes::{ + NoStorageRemoval, SectionedStorageRemoval, +}; use platform_version::version::drive_versions::DriveVersion; const OWNER_ID: [u8; 32] = [0x33; 32]; @@ -124,18 +126,13 @@ fn should_record_the_bucket_owner_when_a_bucket_owned_item_is_deleted_under_v1() ); } -/// The shipped partial batch generation inlines its flag closures instead of -/// calling the crate-delegating entry points, so it parses bucket flags -/// through the shared readers and sections the bytes under the carrier key -/// without recording the owner. Unlike the full batch generation it does not -/// fail closed. This is acceptable because no production path applies a -/// partial batch (only its own module and the version tables name it) and -/// bucket owned bytes are written only from the protocol version whose -/// tables select version 1 of both batch apply methods. This test pins that -/// reasoning: if a caller of the partial batch ever appears, the assertion -/// below is the place that has to change. +/// The shipped partial batch generation is bound to the crate's flags type, +/// which does not know the bucket type bytes, so a bucket-owned removal fails +/// the batch: nothing is applied and no cost operation is pushed. Without +/// this, the bucket's carrier key would reach the shipped fee decoder as a +/// plain sectioned removal and be read as an identity. #[test] -fn should_section_bucket_bytes_without_a_recorded_owner_under_the_shipped_partial_generation() { +fn should_fail_closed_when_a_bucket_owned_item_is_deleted_under_v0() { let shipped = PlatformVersion::latest().drive.clone(); let typed = typed_drive_version(); let owner = RefundOwner::ContractBucket { @@ -146,16 +143,53 @@ fn should_section_bucket_bytes_without_a_recorded_owner_under_the_shipped_partia let drive = setup_drive_with_initial_state_structure(None); insert_flagged_item(&drive, &flags, &typed); + let before = drive + .grove + .root_hash(None, &shipped.grove_version) + .unwrap() + .expect("should get root hash"); - let ops = delete_item_partially(&drive, &shipped).expect("v0 partial applies the delete"); - let [LowLevelDriveOperation::CalculatedCostOperation(cost)] = ops.as_slice() else { - panic!("v0 pushes one plain cost operation, got {:?}", ops); - }; - let SectionedStorageRemoval(removal) = &cost.storage_cost.removed_bytes else { - panic!("an owned delete sections its removed bytes"); - }; - assert_eq!( - removal.keys().copied().collect::>(), - vec![owner.removal_key()] + let mut batch = GroveDbOpBatch::new(); + batch.add_delete(misc_path_vec(), KEY.to_vec()); + let mut drive_operations = vec![]; + let error = drive + .grove_apply_partial_batch_with_add_costs( + batch, + false, + None, + |_cost, _ops_by_level| Ok(vec![]), + &mut drive_operations, + &shipped, + ) + .expect_err("v0 cannot attribute bucket owned bytes"); + + assert!( + error + .to_string() + .contains("unknown storage flags serialization"), + "unexpected error: {}", + error ); + // the read cost incurred before the failure is still pushed, as for any + // failed grove operation, but no removed bytes and no owners reach the + // fee path + for op in &drive_operations { + let LowLevelDriveOperation::CalculatedCostOperation(cost) = op else { + panic!("only plain read costs may be pushed, got {:?}", op); + }; + assert_eq!(cost.storage_cost.removed_bytes, NoStorageRemoval); + } + let after = drive + .grove + .root_hash(None, &shipped.grove_version) + .unwrap() + .expect("should get root hash"); + assert_eq!(after, before, "nothing was applied"); + + // the item is still there and version 1 can still remove it + let ops = delete_item_partially(&drive, &typed).expect("v1 should delete"); + assert!(matches!( + ops.as_slice(), + [LowLevelDriveOperation::CalculatedCostOperationWithRefundOwners { .. }] + )); } diff --git a/packages/rs-drive/src/util/grove_operations/grove_apply_partial_batch_with_add_costs/v0/mod.rs b/packages/rs-drive/src/util/grove_operations/grove_apply_partial_batch_with_add_costs/v0/mod.rs index a736817ab8d..5127e7ee012 100644 --- a/packages/rs-drive/src/util/grove_operations/grove_apply_partial_batch_with_add_costs/v0/mod.rs +++ b/packages/rs-drive/src/util/grove_operations/grove_apply_partial_batch_with_add_costs/v0/mod.rs @@ -6,12 +6,16 @@ use crate::query::GroveError; use crate::util::batch::grovedb_op_batch::GroveDbOpBatchV0Methods; use crate::util::batch::GroveDbOpBatch; use crate::util::grove_operations::push_drive_operation_result; -use crate::util::storage_flags::{MergingOwnersStrategy, StorageFlags}; +// The shipped generation is bound to the crate's flags type on purpose: it +// splits and combines exactly the four historical flag types and rejects any +// other type byte, so bytes owned by a contract credit bucket can never be +// sectioned by this generation. The typed flags live in version 1. use grovedb::batch::{BatchApplyOptions, OpsByLevelPath, QualifiedGroveDbOp}; use grovedb::TransactionArg; use grovedb_costs::storage_cost::removal::StorageRemovedBytes::BasicStorageRemoval; use grovedb_costs::storage_cost::transition::OperationStorageTransitionType; use grovedb_costs::OperationCost; +use grovedb_epoch_based_storage_flags::{MergingOwnersStrategy, StorageFlags}; use platform_version::version::drive_versions::DriveVersion; impl Drive { diff --git a/packages/rs-drive/src/util/storage_flags/mod.rs b/packages/rs-drive/src/util/storage_flags/mod.rs index e354306ce1f..905e5ab9254 100644 --- a/packages/rs-drive/src/util/storage_flags/mod.rs +++ b/packages/rs-drive/src/util/storage_flags/mod.rs @@ -12,16 +12,14 @@ //! bytes paid for by a contract credit bucket. The type byte declares the //! owner's kind; it is never inferred from the shape of an identifier. //! -//! Two groups of entry points differ in what they accept. The parse helpers -//! (`deserialize`, `from_element_flags_ref`, the `map_*` helpers) decode all -//! six variants so that readers carry bucket-owned flags through unchanged. -//! The batch closure entry points that keep their historical names -//! (`update_element_flags`, `split_removal_bytes`) hand the raw bytes to the -//! crate and therefore reject the two bucket variants: the batch apply -//! generation that predates typed owners fails closed on flags it was never -//! written to price. The typed closure entry points -//! (`update_element_flags_typed`, `split_removal_bytes_typed`) accept all -//! six variants and record the owner of every sectioned removal. +//! The parse helpers (`deserialize`, `from_element_flags_ref`, the `map_*` +//! helpers) decode all six variants so that readers carry bucket-owned flags +//! through unchanged. The batch apply generations that predate typed owners +//! do not use this type at all: their closures are bound to the crate's +//! flags type, which rejects the two bucket variants, so they fail closed on +//! flags they were never written to price. The typed closure entry points +//! here (`update_element_flags_typed`, `split_removal_bytes_typed`) accept +//! all six variants and record the owner of every sectioned removal. mod codec; mod combine; diff --git a/packages/rs-drive/src/util/storage_flags/split.rs b/packages/rs-drive/src/util/storage_flags/split.rs index 4e810c139cf..05006ee2999 100644 --- a/packages/rs-drive/src/util/storage_flags/split.rs +++ b/packages/rs-drive/src/util/storage_flags/split.rs @@ -1,14 +1,13 @@ //! Splitting removed bytes across epochs and owners when an element shrinks //! or is deleted. -use super::{CrateStorageFlags, StorageFlags}; +use super::StorageFlags; use crate::error::drive::DriveError; use crate::error::Error; use dpp::fee::refund_owner::{RefundOwner, RefundOwnersByIdentifier, SYSTEM_REFUND_CARRIER_KEY}; use grovedb::ElementFlags; use grovedb_costs::storage_cost::removal::StorageRemovedBytes; use grovedb_costs::storage_cost::removal::StorageRemovedBytes::BasicStorageRemoval; -use grovedb_epoch_based_storage_flags::error::StorageFlagsError; impl StorageFlags { /// Sections removed bytes per epoch, taking from the latest epochs @@ -27,19 +26,6 @@ impl StorageFlags { .split_storage_removed_bytes(removed_key_bytes, removed_value_bytes) } - /// The batch split closure that shipped with identity-only owners. - /// - /// Hands the raw bytes to the crate, so a contract bucket type byte is - /// rejected as an unknown flags type: the batch apply generation that - /// predates typed owners cannot price bytes it cannot attribute. - pub fn split_removal_bytes( - flags: &mut ElementFlags, - removed_key_bytes: u32, - removed_value_bytes: u32, - ) -> Result<(StorageRemovedBytes, StorageRemovedBytes), StorageFlagsError> { - CrateStorageFlags::split_removal_bytes(flags, removed_key_bytes, removed_value_bytes) - } - /// The batch split closure for typed owners. /// /// Decodes all six flag types, sections the removed bytes under the diff --git a/packages/rs-drive/src/util/storage_flags/tests.rs b/packages/rs-drive/src/util/storage_flags/tests.rs index 99d74837eb0..a6f5275e868 100644 --- a/packages/rs-drive/src/util/storage_flags/tests.rs +++ b/packages/rs-drive/src/util/storage_flags/tests.rs @@ -437,11 +437,17 @@ mod split { assert_eq!(refund_owners.get(&owner.removal_key()), Some(&owner)); } + /// The shipped batch apply generations bind their closures to the crate + /// type, so this is the property they rely on. #[test] - fn should_fail_closed_on_bucket_flags_through_the_shipped_closure_entry_points() { + fn should_be_rejected_by_the_crate_closure_entry_points_the_shipped_generations_use() { let mut flags = SingleEpochContractBucket(5, CONTRACT_ID, 7).to_element_flags(); assert!(matches!( - StorageFlags::split_removal_bytes(&mut flags, 1, 1), + CrateStorageFlags::split_removal_bytes(&mut flags, 1, 1), + Err(StorageFlagsError::DeserializeUnknownStorageFlagsType(_)) + )); + assert!(matches!( + CrateStorageFlags::from_element_flags_ref(&flags), Err(StorageFlagsError::DeserializeUnknownStorageFlagsType(_)) )); @@ -452,7 +458,7 @@ mod split { }; let mut new_flags = SingleEpochContractBucket(6, CONTRACT_ID, 7).to_element_flags(); assert!(matches!( - StorageFlags::update_element_flags( + CrateStorageFlags::update_element_flags( &cost, Some(SingleEpochContractBucket(5, CONTRACT_ID, 7).to_element_flags()), &mut new_flags @@ -460,10 +466,10 @@ mod split { Err(StorageFlagsError::DeserializeUnknownStorageFlagsType(_)) )); - // the shipped entry points still work for the historical variants + // the crate still serves the historical variants Drive writes today let mut owned = SingleEpochOwned(5, OWNER_ID).to_element_flags(); let (key_removal, _) = - StorageFlags::split_removal_bytes(&mut owned, 3, 4).expect("should split"); + CrateStorageFlags::split_removal_bytes(&mut owned, 3, 4).expect("should split"); assert_eq!(key_removal, sectioned(OWNER_ID, &[(5, 3)])); } } diff --git a/packages/rs-drive/src/util/storage_flags/update.rs b/packages/rs-drive/src/util/storage_flags/update.rs index 3c621914034..98f33de1342 100644 --- a/packages/rs-drive/src/util/storage_flags/update.rs +++ b/packages/rs-drive/src/util/storage_flags/update.rs @@ -1,6 +1,6 @@ //! Updating the flags of an element that is replaced in a batch. -use super::{CrateStorageFlags, MergingOwnersStrategy, StorageFlags}; +use super::{MergingOwnersStrategy, StorageFlags}; use crate::error::Error; use grovedb::ElementFlags; use grovedb_costs::storage_cost::transition::OperationStorageTransitionType; @@ -8,18 +8,6 @@ use grovedb_costs::storage_cost::StorageCost; use grovedb_epoch_based_storage_flags::error::StorageFlagsError; impl StorageFlags { - /// The batch flag update closure that shipped with identity-only owners. - /// - /// Hands the raw bytes to the crate, so a contract bucket type byte is - /// rejected as an unknown flags type. - pub fn update_element_flags( - cost: &StorageCost, - old_flags: Option, - new_flags: &mut ElementFlags, - ) -> Result { - CrateStorageFlags::update_element_flags(cost, old_flags, new_flags) - } - /// The batch flag update closure for typed owners: the crate's rule /// expressed over the typed flags. /// From f1892049057b8528c75f5dd346803450b8a1e6d2 Mon Sep 17 00:00:00 2001 From: DCG-Claude Date: Wed, 16 Sep 2026 23:35:19 -0500 Subject: [PATCH 06/13] fix(drive): honour the merging strategy when single epoch typed flags shrink later The crate's higher epoch removal combine returns the old flags untouched for single epoch inputs before it looks at the merging strategy, so a bucket owned item replaced by a smaller identity owned payload in a later epoch kept the bucket as its owner under the typed generation while a growing replace transferred. The typed combine now resolves the owner itself on that path with the crate's owner rule (no owner yields, equal keeps, different follows the strategy). A dispatcher test pins the later epoch shrinking replace end to end: the freed bytes refund the bucket, the stored flags name the identity, and the delete afterwards refunds the identity. Also from the review: FeeRefunds::checked_add_assign checks owner conflicts before either map changes so a rejected merge leaves the accumulator intact (test extended); the historical multi epoch variants serialize and size without cloning their epoch map; the bucket carrier key preimage is built on the stack. Refs #4689 Co-Authored-By: Claude Fable 5.1 --- packages/rs-dpp/src/fee/fee_result/refunds.rs | 33 +++---- packages/rs-dpp/src/fee/refund_owner/mod.rs | 13 ++- .../grove_apply_batch_with_add_costs/tests.rs | 94 +++++++++++++++++++ .../rs-drive/src/util/storage_flags/codec.rs | 41 +++++++- .../src/util/storage_flags/combine.rs | 34 +++++++ .../rs-drive/src/util/storage_flags/tests.rs | 76 +++++++++++++++ 6 files changed, 266 insertions(+), 25 deletions(-) diff --git a/packages/rs-dpp/src/fee/fee_result/refunds.rs b/packages/rs-dpp/src/fee/fee_result/refunds.rs index 4fa61dcb7b6..878c5fe2745 100644 --- a/packages/rs-dpp/src/fee/fee_result/refunds.rs +++ b/packages/rs-dpp/src/fee/fee_result/refunds.rs @@ -180,6 +180,20 @@ impl FeeRefunds { /// reported, never resolved by picking one. pub fn checked_add_assign(&mut self, rhs: Self) -> Result<(), ProtocolError> { let Self(rhs_credits, rhs_owners) = rhs; + // owners are checked before either map changes, so a rejected merge + // leaves the accumulator exactly as it was + for (identifier, owner) in &rhs_owners { + if let Some(existing) = self.1.get(identifier) { + if existing != owner { + return Err(ProtocolError::CorruptedCodeExecution(format!( + "storage removal carrier key {} is recorded for two different refund owners: {:?} and {:?}", + hex::encode(identifier), + existing, + owner + ))); + } + } + } for (identifier, mut int_map_b) in rhs_credits.into_iter() { let to_insert_int_map = if let Some(sint_map_a) = self.0.remove(&identifier) { // other has an int_map with the same identifier @@ -202,22 +216,7 @@ impl FeeRefunds { // reinsert the now combined IntMap self.0.insert(identifier, to_insert_int_map); } - for (identifier, owner) in rhs_owners { - match self.1.get(&identifier) { - Some(existing) if *existing != owner => { - return Err(ProtocolError::CorruptedCodeExecution(format!( - "storage removal carrier key {} is recorded for two different refund owners: {:?} and {:?}", - hex::encode(identifier), - existing, - owner - ))); - } - Some(_) => {} - None => { - self.1.insert(identifier, owner); - } - } - } + self.1.extend(rhs_owners); Ok(()) } @@ -541,12 +540,14 @@ mod tests { RefundOwnersByIdentifier::from_iter([(key, bucket)]), ); + let before = refunds.clone(); let result = refunds.checked_add_assign(colliding); assert!(matches!( result, Err(ProtocolError::CorruptedCodeExecution(_)) )); + assert_eq!(refunds, before, "a rejected merge changes nothing"); } } diff --git a/packages/rs-dpp/src/fee/refund_owner/mod.rs b/packages/rs-dpp/src/fee/refund_owner/mod.rs index 5a144f3138f..6f363ad232e 100644 --- a/packages/rs-dpp/src/fee/refund_owner/mod.rs +++ b/packages/rs-dpp/src/fee/refund_owner/mod.rs @@ -37,6 +37,10 @@ pub type ContractCreditBucketPosition = u16; /// the owner's confirmation in the fees workstream register (issue 4689). pub const REFUND_OWNER_CONTRACT_BUCKET_DOMAIN: &[u8; 35] = b"dash-platform/refund-owner/bucket/0"; +/// Size of a contract bucket carrier key preimage: domain, contract id and +/// big-endian position, no length prefixes +const CONTRACT_BUCKET_PREIMAGE_SIZE: usize = 35 + 32 + 2; + /// The system carrier key. Bytes that no owner paid for are sectioned under /// this key and are never refunded, so it is never a recorded owner. pub const SYSTEM_REFUND_CARRIER_KEY: [u8; 32] = [0; 32]; @@ -94,11 +98,10 @@ impl RefundOwner { contract_id, position, } => { - let mut preimage = - Vec::with_capacity(REFUND_OWNER_CONTRACT_BUCKET_DOMAIN.len() + 32 + 2); - preimage.extend_from_slice(REFUND_OWNER_CONTRACT_BUCKET_DOMAIN); - preimage.extend_from_slice(contract_id.as_bytes()); - preimage.extend_from_slice(&position.to_be_bytes()); + let mut preimage = [0u8; CONTRACT_BUCKET_PREIMAGE_SIZE]; + preimage[..35].copy_from_slice(REFUND_OWNER_CONTRACT_BUCKET_DOMAIN); + preimage[35..67].copy_from_slice(contract_id.as_bytes()); + preimage[67..].copy_from_slice(&position.to_be_bytes()); hash_double(preimage) } } diff --git a/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/tests.rs b/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/tests.rs index 0191570aad2..3f93e3bee82 100644 --- a/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/tests.rs +++ b/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/tests.rs @@ -15,12 +15,14 @@ use crate::util::test_helpers::setup::setup_drive_with_initial_state_structure; use dpp::fee::refund_owner::{RefundOwner, RefundOwnersByIdentifier}; use dpp::identifier::Identifier; use dpp::version::PlatformVersion; +use grovedb::batch::QualifiedGroveDbOp; use grovedb::Element; use grovedb_costs::storage_cost::removal::StorageRemovedBytes; use grovedb_costs::storage_cost::removal::StorageRemovedBytes::{ NoStorageRemoval, SectionedStorageRemoval, }; use grovedb_costs::OperationCost; +use grovedb_path::SubtreePath; use platform_version::version::drive_versions::DriveVersion; const OWNER_ID: [u8; 32] = [0x22; 32]; @@ -161,6 +163,98 @@ fn should_record_the_bucket_owner_when_a_bucket_owned_item_is_deleted_under_v1() ); } +/// A replace whose new flags name a different owner transfers the bytes. +/// GroveDB prices a replace with the old flags still attached, so a smaller +/// payload in a later epoch takes the shrinking path, where the crate returns +/// the old single epoch flags untouched. The typed generation must still +/// transfer: the bytes freed by the shrink refund the bucket that paid for +/// them, and the delete afterwards refunds the identity, not the bucket. +#[test] +fn should_transfer_a_bucket_owned_item_to_an_identity_on_a_later_epoch_shrinking_replace() { + let typed = typed_drive_version(); + let bucket = RefundOwner::ContractBucket { + contract_id: Identifier::from(CONTRACT_ID), + position: 7, + }; + let identity = RefundOwner::Identity(Identifier::from(OWNER_ID)); + + let drive = setup_drive_with_initial_state_structure(None); + insert_flagged_item( + &drive, + &StorageFlags::new_single_epoch_for_owner(1, Some(bucket)), + &typed, + ); + + let mut batch = GroveDbOpBatch::new(); + batch.push(QualifiedGroveDbOp::replace_op( + misc_path_vec(), + KEY.to_vec(), + Element::new_item_with_flags( + vec![7u8; 100], + StorageFlags::new_single_epoch_for_owner(2, Some(identity)).to_some_element_flags(), + ), + )); + let mut replace_operations = vec![]; + drive + .grove_apply_batch_with_add_costs(batch, false, None, &mut replace_operations, &typed) + .expect("v1 should replace across kinds"); + + // the bytes freed by the shrink were paid for by the bucket + let [LowLevelDriveOperation::CalculatedCostOperationWithRefundOwners { + cost: shrink_cost, + refund_owners: shrink_owners, + }] = replace_operations.as_slice() + else { + panic!( + "v1 should push one typed cost operation, got {:?}", + replace_operations + ); + }; + let SectionedStorageRemoval(shrink_removal) = sectioned_removal(shrink_cost) else { + panic!("a shrinking replace sections its removed bytes"); + }; + assert_eq!( + shrink_removal.keys().copied().collect::>(), + vec![bucket.removal_key()] + ); + assert_eq!( + *shrink_owners, + RefundOwnersByIdentifier::from([(bucket.removal_key(), bucket)]) + ); + + let stored = drive + .grove + .get( + SubtreePath::from(misc_path_vec().as_slice()), + KEY, + None, + &typed.grove_version, + ) + .unwrap() + .expect("item should exist"); + let flags = StorageFlags::map_some_element_flags_ref(stored.get_flags()) + .expect("flags should decode") + .expect("flags should be present"); + assert_eq!(flags, StorageFlags::SingleEpochOwned(1, OWNER_ID)); + + let ops = delete_item(&drive, &typed).expect("v1 should delete"); + let [LowLevelDriveOperation::CalculatedCostOperationWithRefundOwners { + cost, + refund_owners, + }] = ops.as_slice() + else { + panic!("v1 should push one typed cost operation, got {:?}", ops); + }; + let SectionedStorageRemoval(removal) = sectioned_removal(cost) else { + panic!("an owned delete sections its removed bytes"); + }; + assert_eq!(removal.keys().copied().collect::>(), vec![OWNER_ID]); + assert_eq!( + *refund_owners, + RefundOwnersByIdentifier::from([(OWNER_ID, identity)]) + ); +} + #[test] fn should_fail_closed_when_a_bucket_owned_item_is_deleted_under_v0() { let shipped = PlatformVersion::latest().drive.clone(); diff --git a/packages/rs-drive/src/util/storage_flags/codec.rs b/packages/rs-drive/src/util/storage_flags/codec.rs index c76aa2ba7d0..3846fc50534 100644 --- a/packages/rs-drive/src/util/storage_flags/codec.rs +++ b/packages/rs-drive/src/util/storage_flags/codec.rs @@ -7,7 +7,7 @@ use super::{ BaseEpoch, BytesAddedInEpoch, ContractId, CrateStorageFlags, EpochIndex, StorageFlags, - CONTRACT_BUCKET_OWNER_SIZE, SINGLE_EPOCH_FLAGS_SIZE, + CONTRACT_BUCKET_OWNER_SIZE, OWNER_ID_SIZE, SINGLE_EPOCH_FLAGS_SIZE, }; use dpp::fee::refund_owner::ContractCreditBucketPosition; use grovedb::ElementFlags; @@ -59,8 +59,34 @@ impl StorageFlags { Self::append_epoch_map(&mut buffer, epochs); buffer } - // the four historical variants are exactly the crate's - _ => self.to_crate_flags_keyed_by_removal_key().serialize(), + // the two historical multi epoch variants write the crate's + // layout without cloning their epoch map; equality with the + // crate's bytes is pinned by test + StorageFlags::MultiEpoch(base_epoch, epochs) => { + let mut buffer = Vec::with_capacity( + SINGLE_EPOCH_FLAGS_SIZE as usize + epochs.len() * MIN_EPOCH_MAP_ENTRY_SIZE, + ); + buffer.push(self.type_byte()); + buffer.extend_from_slice(&base_epoch.to_be_bytes()); + Self::append_epoch_map(&mut buffer, epochs); + buffer + } + StorageFlags::MultiEpochOwned(base_epoch, epochs, owner_id) => { + let mut buffer = Vec::with_capacity( + (SINGLE_EPOCH_FLAGS_SIZE + OWNER_ID_SIZE) as usize + + epochs.len() * MIN_EPOCH_MAP_ENTRY_SIZE, + ); + buffer.push(self.type_byte()); + buffer.extend_from_slice(owner_id); + buffer.extend_from_slice(&base_epoch.to_be_bytes()); + Self::append_epoch_map(&mut buffer, epochs); + buffer + } + // the single epoch variants carry no map, so the crate value is + // a plain copy + StorageFlags::SingleEpoch(_) | StorageFlags::SingleEpochOwned(..) => { + self.to_crate_flags_keyed_by_removal_key().serialize() + } } } @@ -71,7 +97,14 @@ impl StorageFlags { StorageFlags::MultiEpochContractBucket(_, epochs, ..) => { CONTRACT_BUCKET_HEADER_SIZE as u32 + Self::epoch_map_size(epochs) } - _ => self.to_crate_flags_keyed_by_removal_key().serialized_size(), + StorageFlags::MultiEpoch(_, epochs) => { + SINGLE_EPOCH_FLAGS_SIZE + Self::epoch_map_size(epochs) + } + StorageFlags::MultiEpochOwned(_, epochs, _) => { + SINGLE_EPOCH_FLAGS_SIZE + OWNER_ID_SIZE + Self::epoch_map_size(epochs) + } + StorageFlags::SingleEpoch(_) => SINGLE_EPOCH_FLAGS_SIZE, + StorageFlags::SingleEpochOwned(..) => SINGLE_EPOCH_FLAGS_SIZE + OWNER_ID_SIZE, } } diff --git a/packages/rs-drive/src/util/storage_flags/combine.rs b/packages/rs-drive/src/util/storage_flags/combine.rs index 7f331befef0..00551e49c1a 100644 --- a/packages/rs-drive/src/util/storage_flags/combine.rs +++ b/packages/rs-drive/src/util/storage_flags/combine.rs @@ -62,6 +62,12 @@ impl StorageFlags { } /// Combine removed bytes + /// + /// When a single epoch element shrinks in a later epoch there is no epoch + /// map to subtract from and the crate returns the old flags untouched, + /// before it looks at the merging strategy. The typed path resolves the + /// owner itself in that case so that `UseTheirs` transfers ownership on a + /// shrinking replace exactly as it does on a growing one. pub fn combine_removed_bytes( self, rhs: Self, @@ -71,6 +77,10 @@ impl StorageFlags { let ours_owner = self.refund_owner(); let theirs_owner = rhs.refund_owner(); Self::reject_colliding_owners(ours_owner, theirs_owner)?; + if self.epoch_index_map().is_none() && self.base_epoch() < rhs.base_epoch() { + let owner = Self::resolve_owner(ours_owner, theirs_owner, merging_owners_strategy)?; + return Ok(Self::new_single_epoch_for_owner(*self.base_epoch(), owner)); + } let combined = self .to_crate_flags_keyed_by_removal_key() .combine_removed_bytes( @@ -81,6 +91,30 @@ impl StorageFlags { Self::from_crate_combined(combined, ours_owner, theirs_owner) } + /// The crate's owner rule over typed owners: a side without an owner + /// yields to the other, equal owners keep, different owners follow the + /// strategy. + fn resolve_owner( + ours: Option, + theirs: Option, + merging_owners_strategy: MergingOwnersStrategy, + ) -> Result, StorageFlagsError> { + match (ours, theirs) { + (None, theirs) => Ok(theirs), + (ours, None) => Ok(ours), + (Some(ours), Some(theirs)) if ours == theirs => Ok(Some(ours)), + (Some(ours), Some(theirs)) => match merging_owners_strategy { + MergingOwnersStrategy::RaiseIssue => { + Err(StorageFlagsError::MergingStorageFlagsFromDifferentOwners( + "can not merge from different owners".to_string(), + )) + } + MergingOwnersStrategy::UseOurs => Ok(Some(ours)), + MergingOwnersStrategy::UseTheirs => Ok(Some(theirs)), + }, + } + } + /// Two different typed owners with one carrier key can never be told /// apart by the crate, so the merge is refused before it starts. fn reject_colliding_owners( diff --git a/packages/rs-drive/src/util/storage_flags/tests.rs b/packages/rs-drive/src/util/storage_flags/tests.rs index a6f5275e868..61b91c71cb6 100644 --- a/packages/rs-drive/src/util/storage_flags/tests.rs +++ b/packages/rs-drive/src/util/storage_flags/tests.rs @@ -599,6 +599,82 @@ mod combine { ); } + /// A single epoch element that shrinks in a later epoch has no epoch map + /// to subtract from. The crate returns the old flags before it looks at + /// the strategy; the typed path must still honour it, or a shrinking + /// replace would keep the old owner while a growing one transfers. + #[test] + fn should_transfer_ownership_across_kinds_when_a_single_epoch_element_shrinks_later() { + let identity = RefundOwner::Identity(Identifier::from(OWNER_ID)); + let removed = sectioned(identity.removal_key(), &[(1, 2)]); + + let bucket_to_identity = SingleEpochContractBucket(1, CONTRACT_ID, 7) + .combine_removed_bytes( + SingleEpochOwned(2, OWNER_ID), + &removed, + MergingOwnersStrategy::UseTheirs, + ) + .expect("should combine"); + assert_eq!(bucket_to_identity, SingleEpochOwned(1, OWNER_ID)); + + let identity_to_bucket = SingleEpochOwned(1, OWNER_ID) + .combine_removed_bytes( + SingleEpochContractBucket(2, CONTRACT_ID, 7), + &removed, + MergingOwnersStrategy::UseTheirs, + ) + .expect("should combine"); + assert_eq!( + identity_to_bucket, + SingleEpochContractBucket(1, CONTRACT_ID, 7) + ); + + let kept_ours = SingleEpochContractBucket(1, CONTRACT_ID, 7) + .combine_removed_bytes( + SingleEpochOwned(2, OWNER_ID), + &removed, + MergingOwnersStrategy::UseOurs, + ) + .expect("should combine"); + assert_eq!(kept_ours, SingleEpochContractBucket(1, CONTRACT_ID, 7)); + + assert!(matches!( + SingleEpochContractBucket(1, CONTRACT_ID, 7).combine_removed_bytes( + SingleEpochOwned(2, OWNER_ID), + &removed, + MergingOwnersStrategy::RaiseIssue, + ), + Err(StorageFlagsError::MergingStorageFlagsFromDifferentOwners(_)) + )); + + // unowned on either side yields to the owned side, as in the crate + let gained = SingleEpoch(1) + .combine_removed_bytes( + SingleEpochContractBucket(2, CONTRACT_ID, 7), + &removed, + MergingOwnersStrategy::RaiseIssue, + ) + .expect("should combine"); + assert_eq!(gained, SingleEpochContractBucket(1, CONTRACT_ID, 7)); + + // the same owner on both sides is unchanged, matching the crate + let same = SingleEpochOwned(1, OWNER_ID) + .combine_removed_bytes( + SingleEpochOwned(2, OWNER_ID), + &removed, + MergingOwnersStrategy::UseTheirs, + ) + .expect("should combine"); + let crate_same = CrateStorageFlags::SingleEpochOwned(1, OWNER_ID) + .combine_removed_bytes( + CrateStorageFlags::SingleEpochOwned(2, OWNER_ID), + &removed, + MergingOwnersStrategy::UseTheirs, + ) + .expect("should combine"); + assert_eq!(same, StorageFlags::from(crate_same)); + } + #[test] fn should_keep_the_typed_owner_when_removing_bytes_in_a_higher_epoch() { let owner = bucket_owner(7); From af9ea02c018dfef926c6a6af3d0b11813ed98ec9 Mon Sep 17 00:00:00 2001 From: DCG-Claude Date: Thu, 17 Sep 2026 01:53:31 -0500 Subject: [PATCH 07/13] fix(drive): re-price and converge cross-kind replaces under the typed flag update GroveDB prices a replace with the old flags attached and re-prices only when the flag update closure reports a change. Two cases broke under the typed generation. A same-epoch replace across owner kinds leaves the proposed flags equal to the merged flags while the header grows or shrinks by two bytes, so the closure reported no change and GroveDB rejected the write with a storage cost mismatch. A later-epoch replace whose payload shrinks by exactly the header growth is priced as a shrink first and as same size on the second pass; the same-size branch restored the old flags, undoing the transfer the first pass chose, and the two passes alternated until GroveDB's cyclic guard fired. The typed closure now reports a change whenever the combined flags differ in width from the old flags, and its same-size branch keeps our epochs while resolving the owner by the same rule as the added and removed branches. The combine adapters move epoch maps instead of cloning them. Dispatcher tests pin both shapes in both directions. The shipped generation 0 closures are the crate's and unchanged. Also from the review: FeeRefunds::checked_add_assign refuses to merge credits that lack a recorded owner on either side, so a merge can never lend an owner to unowned credits; ensure_identity_owners_only guards the identity keyed accessors for new callers, and a Drive test pins that the shipped balance consumer halts on a bucket owned refund rather than crediting anyone. Refs #4689 Co-Authored-By: Claude Fable 5.1 --- packages/rs-dpp/src/fee/fee_result/mod.rs | 7 +- packages/rs-dpp/src/fee/fee_result/refunds.rs | 147 +++++++++++++- .../src/drive/identity/balance/update.rs | 56 ++++++ .../grove_apply_batch_with_add_costs/tests.rs | 185 ++++++++++++++++++ .../src/util/storage_flags/combine.rs | 58 +++--- .../rs-drive/src/util/storage_flags/mod.rs | 70 +++++++ .../rs-drive/src/util/storage_flags/tests.rs | 67 ++++++- .../rs-drive/src/util/storage_flags/update.rs | 50 +++-- 8 files changed, 599 insertions(+), 41 deletions(-) diff --git a/packages/rs-dpp/src/fee/fee_result/mod.rs b/packages/rs-dpp/src/fee/fee_result/mod.rs index b753b1f7d2b..050a67d1f65 100644 --- a/packages/rs-dpp/src/fee/fee_result/mod.rs +++ b/packages/rs-dpp/src/fee/fee_result/mod.rs @@ -131,7 +131,12 @@ impl BalanceChangeForIdentity { &self.change } - /// Returns refund amount of credits for other identities + /// Returns refund amount of credits for other identities. + /// + /// Identity keyed view kept for the shipped balance consumer, which runs + /// only under generations that predate typed owners. A path that may + /// hold bucket owned refunds uses [`Self::other_typed_refunds`], or + /// checks `FeeRefunds::ensure_identity_owners_only` first. pub fn other_refunds(&self) -> BTreeMap { self.fee_result .fee_refunds diff --git a/packages/rs-dpp/src/fee/fee_result/refunds.rs b/packages/rs-dpp/src/fee/fee_result/refunds.rs index 878c5fe2745..6fa5c60d0cb 100644 --- a/packages/rs-dpp/src/fee/fee_result/refunds.rs +++ b/packages/rs-dpp/src/fee/fee_result/refunds.rs @@ -182,6 +182,33 @@ impl FeeRefunds { let Self(rhs_credits, rhs_owners) = rhs; // owners are checked before either map changes, so a rejected merge // leaves the accumulator exactly as it was + for identifier in rhs_credits.keys() { + let Some(owner) = rhs_owners.get(identifier) else { + return Err(ProtocolError::CorruptedCodeExecution(format!( + "storage refund carrier key {} has no recorded refund owner", + hex::encode(identifier) + ))); + }; + match self.1.get(identifier) { + Some(existing) if existing != owner => { + return Err(ProtocolError::CorruptedCodeExecution(format!( + "storage removal carrier key {} is recorded for two different refund owners: {:?} and {:?}", + hex::encode(identifier), + existing, + owner + ))); + } + // credits already held without an owner must not acquire + // one from the other side + None if self.0.contains_key(identifier) => { + return Err(ProtocolError::CorruptedCodeExecution(format!( + "storage refund carrier key {} has no recorded refund owner", + hex::encode(identifier) + ))); + } + _ => {} + } + } for (identifier, owner) in &rhs_owners { if let Some(existing) = self.1.get(identifier) { if existing != owner { @@ -273,7 +300,36 @@ impl FeeRefunds { summed_credits } - /// Calculates a refund amount of credits per identity excluding specified identity id + /// Checks that every recorded owner is an identity, so that the identity + /// keyed accessors below can be used without reading a bucket's carrier + /// key as an identity id. A carrier key without a recorded owner fails + /// the same way. + /// + /// The identity keyed accessors keep their historical signatures because + /// the shipped balance consumer calls them; that consumer runs only under + /// generations that predate typed owners, and Drive fails closed there on + /// a carrier key that has no identity balance. New callers check this + /// guard first or use the typed accessors. + pub fn ensure_identity_owners_only(&self) -> Result<(), ProtocolError> { + for entry in self.iter_typed() { + let (owner, identifier, _) = entry?; + if owner.as_identity().is_none() { + return Err(ProtocolError::CorruptedCodeExecution(format!( + "storage refund carrier key {} belongs to {:?}, not to an identity", + hex::encode(identifier), + owner + ))); + } + } + Ok(()) + } + + /// Calculates a refund amount of credits per identity excluding specified identity id. + /// + /// Identity keyed view: every carrier key is returned as an identity id. + /// Call [`Self::ensure_identity_owners_only`] first on a path that may + /// hold bucket owned refunds, or use + /// [`Self::calculate_all_refunds_except_owner`]. pub fn calculate_all_refunds_except_identity( &self, identity_id: Identifier, @@ -321,7 +377,11 @@ impl FeeRefunds { Ok(refunds_by_owner) } - /// Calculates a refund amount of credits for specified identity id + /// Calculates a refund amount of credits for specified identity id. + /// + /// Identity keyed view: looks the identity id up as a carrier key. An + /// identity id can never equal a bucket's carrier key except by a hash + /// preimage, so this stays exact for identity payers. pub fn calculate_refunds_amount_for_identity( &self, identity_id: Identifier, @@ -549,6 +609,36 @@ mod tests { )); assert_eq!(refunds, before, "a rejected merge changes nothing"); } + + #[test] + fn should_reject_a_merge_that_would_give_unowned_credits_an_owner() { + let bucket = bucket_owner(2, 0); + let key = bucket.removal_key(); + let unowned = FeeRefunds( + CreditsPerEpochByIdentifier::from_iter([( + key, + CreditsPerEpoch::from_iter([(0, 10)]), + )]), + RefundOwnersByIdentifier::new(), + ); + let owned = refunds_for(bucket, 0, 1); + + let mut left = unowned.clone(); + let before = left.clone(); + assert!(matches!( + left.checked_add_assign(owned.clone()), + Err(ProtocolError::CorruptedCodeExecution(_)) + )); + assert_eq!(left, before, "a rejected merge changes nothing"); + + let mut right = owned; + let before = right.clone(); + assert!(matches!( + right.checked_add_assign(unowned), + Err(ProtocolError::CorruptedCodeExecution(_)) + )); + assert_eq!(right, before, "a rejected merge changes nothing"); + } } mod typed_accessors { @@ -614,6 +704,59 @@ mod tests { )); } + #[test] + fn should_guard_the_identity_keyed_view_against_bucket_owned_refunds() { + let identity = RefundOwner::Identity(Identifier::from([1u8; 32])); + let bucket = bucket_owner(3, 4); + let owners = RefundOwnersByIdentifier::from_iter([ + (identity.removal_key(), identity), + (bucket.removal_key(), bucket), + ]); + let removal = BytesPerEpochByIdentifier::from_iter([ + (identity.removal_key(), IntMap::from_iter([(0, 100)])), + (bucket.removal_key(), IntMap::from_iter([(0, 200)])), + ]); + let typed = FeeRefunds::from_typed_storage_removal( + removal, + &owners, + 3, + 20, + &EPOCH_CHANGE_FEE_VERSION_TEST, + ) + .expect("should create fee refunds"); + + assert!(matches!( + typed.ensure_identity_owners_only(), + Err(ProtocolError::CorruptedCodeExecution(_)) + )); + + let identities_only = FeeRefunds::from_storage_removal( + BytesPerEpochByIdentifier::from_iter([( + identity.removal_key(), + IntMap::from_iter([(0, 100)]), + )]), + 3, + 20, + &EPOCH_CHANGE_FEE_VERSION_TEST, + ) + .expect("should create fee refunds"); + identities_only + .ensure_identity_owners_only() + .expect("identity owners pass"); + + let unowned = FeeRefunds( + CreditsPerEpochByIdentifier::from_iter([( + [4u8; 32], + CreditsPerEpoch::from_iter([(0, 100)]), + )]), + RefundOwnersByIdentifier::new(), + ); + assert!(matches!( + unowned.ensure_identity_owners_only(), + Err(ProtocolError::CorruptedCodeExecution(_)) + )); + } + #[test] fn should_sum_per_epoch_across_owners_of_both_kinds() { let identity = RefundOwner::Identity(Identifier::from([1u8; 32])); diff --git a/packages/rs-drive/src/drive/identity/balance/update.rs b/packages/rs-drive/src/drive/identity/balance/update.rs index b3226a0d2ca..49477812d27 100644 --- a/packages/rs-drive/src/drive/identity/balance/update.rs +++ b/packages/rs-drive/src/drive/identity/balance/update.rs @@ -531,6 +531,62 @@ mod tests { use nohash_hasher::IntMap; use std::collections::BTreeMap; + /// The shipped consumer reads refund carrier keys as identity ids. + /// A bucket owned refund can only reach it under a generation that + /// predates typed owners; it must halt there, never credit anyone. + #[test] + fn should_fail_closed_on_a_bucket_owned_refund() { + let drive = setup_drive_with_initial_state_structure(None); + + let platform_version = PlatformVersion::latest(); + + let identity = create_test_identity(&drive, [0; 32], Some(15), None, platform_version) + .expect("expected to create an identity"); + let bucket = RefundOwner::ContractBucket { + contract_id: [7; 32].into(), + position: 1, + }; + + // the payer's own refund keeps its balance change non-zero, so + // the consumer goes on to the other refunds instead of + // returning early + let refunds_per_epoch_by_identifier: CreditsPerEpochByIdentifier = + BTreeMap::from_iter([ + ( + identity.id().to_buffer(), + IntMap::from_iter([(GENESIS_EPOCH_INDEX, 100000)]), + ), + ( + bucket.removal_key(), + IntMap::from_iter([(GENESIS_EPOCH_INDEX, 200000)]), + ), + ]); + let refund_owners = BTreeMap::from_iter([ + ( + identity.id().to_buffer(), + RefundOwner::Identity(identity.id()), + ), + (bucket.removal_key(), bucket), + ]); + + let fee_result = FeeResult { + fee_refunds: FeeRefunds(refunds_per_epoch_by_identifier, refund_owners), + ..Default::default() + }; + let fee_change = fee_result.into_balance_change(identity.id()); + + let result = drive.apply_balance_change_from_fee_to_identity_operations( + fee_change, + None, + platform_version, + ); + + assert!(matches!( + result, + Err(Error::Drive(DriveError::CorruptedCodeExecution(_))) + )); + } + #[test] fn should_do_nothing_if_there_is_no_balance_change() { let drive = setup_drive_with_initial_state_structure(None); diff --git a/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/tests.rs b/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/tests.rs index 3f93e3bee82..cbe93765a13 100644 --- a/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/tests.rs +++ b/packages/rs-drive/src/util/grove_operations/grove_apply_batch_with_add_costs/tests.rs @@ -255,6 +255,191 @@ fn should_transfer_a_bucket_owned_item_to_an_identity_on_a_later_epoch_shrinking ); } +/// Replaces `KEY` under the typed generation with `payload_len` bytes and +/// the given flags, returning the pushed operations. +fn replace_item( + drive: &Drive, + payload_len: usize, + flags: &StorageFlags, + drive_version: &DriveVersion, +) -> Result, Error> { + let mut batch = GroveDbOpBatch::new(); + batch.push(QualifiedGroveDbOp::replace_op( + misc_path_vec(), + KEY.to_vec(), + Element::new_item_with_flags(vec![7u8; payload_len], flags.to_some_element_flags()), + )); + let mut drive_operations = vec![]; + drive.grove_apply_batch_with_add_costs( + batch, + false, + None, + &mut drive_operations, + drive_version, + )?; + Ok(drive_operations) +} + +fn stored_flags(drive: &Drive, drive_version: &DriveVersion) -> StorageFlags { + let stored = drive + .grove + .get( + SubtreePath::from(misc_path_vec().as_slice()), + KEY, + None, + &drive_version.grove_version, + ) + .unwrap() + .expect("item should exist"); + StorageFlags::map_some_element_flags_ref(stored.get_flags()) + .expect("flags should decode") + .expect("flags should be present") +} + +/// GroveDB prices a replace with the old flags attached and asks the update +/// closure whether the flags changed. A same-epoch replace across kinds +/// leaves the proposed flags as they are but changes the header width (35 +/// bytes for an identity, 37 for a bucket), so the closure must report a +/// change or GroveDB keeps a stale price and rejects the write. +#[test] +fn should_replace_across_kinds_in_the_same_epoch_when_the_header_width_changes() { + let typed = typed_drive_version(); + let bucket = RefundOwner::ContractBucket { + contract_id: Identifier::from(CONTRACT_ID), + position: 7, + }; + let identity = RefundOwner::Identity(Identifier::from(OWNER_ID)); + + // identity to bucket, payload one byte bigger + let drive = setup_drive_with_initial_state_structure(None); + insert_flagged_item( + &drive, + &StorageFlags::new_single_epoch_for_owner(1, Some(identity)), + &typed, + ); + let ops = replace_item( + &drive, + 201, + &StorageFlags::new_single_epoch_for_owner(1, Some(bucket)), + &typed, + ) + .expect("identity to bucket replace should apply"); + assert!(matches!( + ops.as_slice(), + [LowLevelDriveOperation::CalculatedCostOperationWithRefundOwners { .. }] + )); + assert_eq!( + stored_flags(&drive, &typed), + StorageFlags::SingleEpochContractBucket(1, CONTRACT_ID, 7) + ); + let ops = delete_item(&drive, &typed).expect("v1 should delete"); + let [LowLevelDriveOperation::CalculatedCostOperationWithRefundOwners { refund_owners, .. }] = + ops.as_slice() + else { + panic!("v1 should push one typed cost operation, got {:?}", ops); + }; + assert_eq!( + *refund_owners, + RefundOwnersByIdentifier::from([(bucket.removal_key(), bucket)]) + ); + + // bucket to identity, payload one byte bigger: priced with the old + // header it looks like growth, with the new header it is a one byte + // shrink, and the freed byte belongs to the bucket that paid for it + let drive = setup_drive_with_initial_state_structure(None); + insert_flagged_item( + &drive, + &StorageFlags::new_single_epoch_for_owner(1, Some(bucket)), + &typed, + ); + let ops = replace_item( + &drive, + 201, + &StorageFlags::new_single_epoch_for_owner(1, Some(identity)), + &typed, + ) + .expect("bucket to identity replace should apply"); + let [LowLevelDriveOperation::CalculatedCostOperationWithRefundOwners { + cost, + refund_owners, + }] = ops.as_slice() + else { + panic!("v1 should push one typed cost operation, got {:?}", ops); + }; + let SectionedStorageRemoval(removal) = sectioned_removal(cost) else { + panic!("the one byte shrink is sectioned, got {:?}", cost); + }; + assert_eq!( + removal.keys().copied().collect::>(), + vec![bucket.removal_key()] + ); + assert_eq!( + *refund_owners, + RefundOwnersByIdentifier::from([(bucket.removal_key(), bucket)]) + ); + assert_eq!( + stored_flags(&drive, &typed), + StorageFlags::SingleEpochOwned(1, OWNER_ID) + ); +} + +/// A later-epoch replace whose payload shrinks by exactly the header growth +/// of the new owner kind nets to zero bytes. The first pricing pass sees a +/// shrink and transfers ownership; the recalculated pass sees the same size +/// and must resolve ownership the same way, or GroveDB's update loop +/// alternates between the two answers and never converges. +#[test] +fn should_converge_when_a_later_epoch_replace_nets_to_the_same_size_across_kinds() { + let typed = typed_drive_version(); + let bucket = RefundOwner::ContractBucket { + contract_id: Identifier::from(CONTRACT_ID), + position: 7, + }; + let identity = RefundOwner::Identity(Identifier::from(OWNER_ID)); + + let drive = setup_drive_with_initial_state_structure(None); + insert_flagged_item( + &drive, + &StorageFlags::new_single_epoch_for_owner(1, Some(identity)), + &typed, + ); + let ops = replace_item( + &drive, + 198, + &StorageFlags::new_single_epoch_for_owner(2, Some(bucket)), + &typed, + ) + .expect("a net same size replace across kinds should apply"); + + let [LowLevelDriveOperation::CalculatedCostOperationWithRefundOwners { cost, .. }] = + ops.as_slice() + else { + panic!("v1 should push one typed cost operation, got {:?}", ops); + }; + assert_eq!(cost.storage_cost.added_bytes, 0, "nothing was added on net"); + assert_eq!( + *sectioned_removal(cost), + NoStorageRemoval, + "nothing was freed on net" + ); + // the bytes keep their original epoch and move to the new owner + assert_eq!( + stored_flags(&drive, &typed), + StorageFlags::SingleEpochContractBucket(1, CONTRACT_ID, 7) + ); + + let ops = delete_item(&drive, &typed).expect("v1 should delete"); + let [LowLevelDriveOperation::CalculatedCostOperationWithRefundOwners { refund_owners, .. }] = + ops.as_slice() + else { + panic!("v1 should push one typed cost operation, got {:?}", ops); + }; + assert_eq!( + *refund_owners, + RefundOwnersByIdentifier::from([(bucket.removal_key(), bucket)]) + ); +} + #[test] fn should_fail_closed_when_a_bucket_owned_item_is_deleted_under_v0() { let shipped = PlatformVersion::latest().drive.clone(); diff --git a/packages/rs-drive/src/util/storage_flags/combine.rs b/packages/rs-drive/src/util/storage_flags/combine.rs index 00551e49c1a..7a81e2b9a62 100644 --- a/packages/rs-drive/src/util/storage_flags/combine.rs +++ b/packages/rs-drive/src/util/storage_flags/combine.rs @@ -52,9 +52,9 @@ impl StorageFlags { let theirs_owner = rhs.refund_owner(); Self::reject_colliding_owners(ours_owner, theirs_owner)?; let combined = self - .to_crate_flags_keyed_by_removal_key() + .into_crate_flags_keyed_by_removal_key() .combine_added_bytes( - rhs.to_crate_flags_keyed_by_removal_key(), + rhs.into_crate_flags_keyed_by_removal_key(), added_bytes, merging_owners_strategy, )?; @@ -82,15 +82,36 @@ impl StorageFlags { return Ok(Self::new_single_epoch_for_owner(*self.base_epoch(), owner)); } let combined = self - .to_crate_flags_keyed_by_removal_key() + .into_crate_flags_keyed_by_removal_key() .combine_removed_bytes( - rhs.to_crate_flags_keyed_by_removal_key(), + rhs.into_crate_flags_keyed_by_removal_key(), removed_bytes, merging_owners_strategy, )?; Self::from_crate_combined(combined, ours_owner, theirs_owner) } + /// Combine for a replace that moved no bytes: the epochs stay ours and + /// the owner follows the merging strategy. + /// + /// This is the branch GroveDB reaches when a replace nets to the same + /// size. It must resolve ownership exactly as the added and removed + /// branches do, because GroveDB may price one replace as a shrink first + /// and as same size after the flags changed width; two different answers + /// would never converge. + pub fn combine_same_size( + self, + rhs: Self, + merging_owners_strategy: MergingOwnersStrategy, + ) -> Result { + let ours_owner = self.refund_owner(); + let theirs_owner = rhs.refund_owner(); + Self::reject_colliding_owners(ours_owner, theirs_owner)?; + let owner = Self::resolve_owner(ours_owner, theirs_owner, merging_owners_strategy)?; + let (base_epoch, epochs, _) = self.into_parts(); + Ok(Self::from_parts(base_epoch, epochs, owner)) + } + /// The crate's owner rule over typed owners: a side without an owner /// yields to the other, equal owners keep, different owners follow the /// strategy. @@ -150,27 +171,14 @@ impl StorageFlags { "combined storage flags name an owner that neither side supplied".to_string(), ) })?; - match owner { - RefundOwner::Identity(_) => Ok(Self::from(combined)), - RefundOwner::ContractBucket { - contract_id, - position, - } => { - let base_epoch = *combined.base_epoch(); - Ok(match combined.epoch_index_map() { - None => StorageFlags::SingleEpochContractBucket( - base_epoch, - contract_id.to_buffer(), - position, - ), - Some(epochs) => StorageFlags::MultiEpochContractBucket( - base_epoch, - epochs.clone(), - contract_id.to_buffer(), - position, - ), - }) + let (base_epoch, epochs) = match combined { + CrateStorageFlags::SingleEpoch(base_epoch) + | CrateStorageFlags::SingleEpochOwned(base_epoch, _) => (base_epoch, None), + CrateStorageFlags::MultiEpoch(base_epoch, epochs) + | CrateStorageFlags::MultiEpochOwned(base_epoch, epochs, _) => { + (base_epoch, Some(epochs)) } - } + }; + Ok(Self::from_parts(base_epoch, epochs, Some(owner))) } } diff --git a/packages/rs-drive/src/util/storage_flags/mod.rs b/packages/rs-drive/src/util/storage_flags/mod.rs index 905e5ab9254..b0a11f0e951 100644 --- a/packages/rs-drive/src/util/storage_flags/mod.rs +++ b/packages/rs-drive/src/util/storage_flags/mod.rs @@ -328,6 +328,76 @@ impl StorageFlags { Some(Cow::Owned(self)) } + /// Builds the flags for a base epoch, an optional epoch map and an + /// optional typed owner. An empty epoch map gives a single epoch variant. + pub(super) fn from_parts( + base_epoch: BaseEpoch, + epochs: Option>, + owner: Option, + ) -> Self { + let epochs = epochs.filter(|epochs| !epochs.is_empty()); + match (owner, epochs) { + (None, None) => SingleEpoch(base_epoch), + (None, Some(epochs)) => MultiEpoch(base_epoch, epochs), + (Some(RefundOwner::Identity(identity_id)), None) => { + SingleEpochOwned(base_epoch, identity_id.to_buffer()) + } + (Some(RefundOwner::Identity(identity_id)), Some(epochs)) => { + MultiEpochOwned(base_epoch, epochs, identity_id.to_buffer()) + } + ( + Some(RefundOwner::ContractBucket { + contract_id, + position, + }), + None, + ) => SingleEpochContractBucket(base_epoch, contract_id.to_buffer(), position), + ( + Some(RefundOwner::ContractBucket { + contract_id, + position, + }), + Some(epochs), + ) => MultiEpochContractBucket(base_epoch, epochs, contract_id.to_buffer(), position), + } + } + + /// Splits the flags into their base epoch, epoch map and typed owner, + /// moving the map out. + pub(super) fn into_parts( + self, + ) -> ( + BaseEpoch, + Option>, + Option, + ) { + let owner = self.refund_owner(); + match self { + SingleEpoch(base_epoch) + | SingleEpochOwned(base_epoch, _) + | SingleEpochContractBucket(base_epoch, ..) => (base_epoch, None, owner), + MultiEpoch(base_epoch, epochs) + | MultiEpochOwned(base_epoch, epochs, _) + | MultiEpochContractBucket(base_epoch, epochs, ..) => (base_epoch, Some(epochs), owner), + } + } + + /// The crate's value with every typed owner replaced by its removal key, + /// moving the epoch map instead of cloning it. See + /// [`Self::to_crate_flags_keyed_by_removal_key`] for why this is sound. + pub(super) fn into_crate_flags_keyed_by_removal_key(self) -> CrateStorageFlags { + let (base_epoch, epochs, owner) = self.into_parts(); + let key = owner.map(|owner| owner.removal_key()); + match (key, epochs) { + (None, None) => CrateStorageFlags::SingleEpoch(base_epoch), + (None, Some(epochs)) => CrateStorageFlags::MultiEpoch(base_epoch, epochs), + (Some(key), None) => CrateStorageFlags::SingleEpochOwned(base_epoch, key), + (Some(key), Some(epochs)) => { + CrateStorageFlags::MultiEpochOwned(base_epoch, epochs, key) + } + } + } + /// The crate's value with every typed owner replaced by its removal key. /// /// This is what lets the crate's epoch arithmetic (split and combine) run diff --git a/packages/rs-drive/src/util/storage_flags/tests.rs b/packages/rs-drive/src/util/storage_flags/tests.rs index 61b91c71cb6..f4adc2020b2 100644 --- a/packages/rs-drive/src/util/storage_flags/tests.rs +++ b/packages/rs-drive/src/util/storage_flags/tests.rs @@ -784,11 +784,74 @@ mod update { StorageFlags::update_element_flags_typed(&bigger(10), Some(old), &mut new_flags) .expect("should update"); - assert!(!changed, "new flags already name the new owner"); + assert!( + changed, + "the owner already matches but the header grew from 35 to 37 bytes, so \ + GroveDB has to price the element again" + ); assert_eq!( StorageFlags::deserialize(&new_flags).expect("should decode"), Some(SingleEpochContractBucket(1, CONTRACT_ID, 7)) ); + + // the same owner kind on both sides keeps the width, so an unchanged + // merge is reported as unchanged + let old = SingleEpochOwned(1, OWNER_ID).to_element_flags(); + let mut same_kind = SingleEpochOwned(1, OWNER_ID).to_element_flags(); + let changed = + StorageFlags::update_element_flags_typed(&bigger(10), Some(old), &mut same_kind) + .expect("should update"); + assert!(!changed); + } + + /// GroveDB may price one replace as a shrink first and, once the flags + /// changed width, as same size. Both passes must name the same owner. + #[test] + fn should_resolve_a_same_size_update_the_same_way_as_a_shrink() { + let identity = RefundOwner::Identity(Identifier::from(OWNER_ID)); + let old = SingleEpochOwned(1, OWNER_ID).to_element_flags(); + let proposed = SingleEpochContractBucket(2, CONTRACT_ID, 7).to_element_flags(); + + let shrink = StorageCost { + added_bytes: 0, + replaced_bytes: 1, + removed_bytes: sectioned(identity.removal_key(), &[(1, 2)]), + }; + let mut after_shrink = proposed.clone(); + StorageFlags::update_element_flags_typed(&shrink, Some(old.clone()), &mut after_shrink) + .expect("should update"); + + let same_size = StorageCost { + added_bytes: 0, + replaced_bytes: 1, + removed_bytes: NoStorageRemoval, + }; + let mut after_same_size = proposed; + let changed = + StorageFlags::update_element_flags_typed(&same_size, Some(old), &mut after_same_size) + .expect("should update"); + + assert!(changed); + assert_eq!(after_shrink, after_same_size); + assert_eq!( + StorageFlags::deserialize(&after_same_size).expect("should decode"), + Some(SingleEpochContractBucket(1, CONTRACT_ID, 7)) + ); + + // a multi epoch element keeps its whole map on a same size update + let old = MultiEpochOwned(1, epochs(&[(2, 20)]), OWNER_ID).to_element_flags(); + let mut new_flags = SingleEpochContractBucket(3, CONTRACT_ID, 7).to_element_flags(); + StorageFlags::update_element_flags_typed(&same_size, Some(old), &mut new_flags) + .expect("should update"); + assert_eq!( + StorageFlags::deserialize(&new_flags).expect("should decode"), + Some(MultiEpochContractBucket( + 1, + epochs(&[(2, 20)]), + CONTRACT_ID, + 7 + )) + ); } #[test] @@ -819,7 +882,7 @@ mod update { } #[test] - fn should_keep_old_flags_on_a_same_size_update_and_pass_through_inserts() { + fn should_keep_old_epochs_on_a_same_size_update_and_pass_through_inserts() { let old = SingleEpochContractBucket(9, CONTRACT_ID, 7).to_element_flags(); let mut new_flags = SingleEpochContractBucket(1, CONTRACT_ID, 7).to_element_flags(); let same_size = StorageCost { diff --git a/packages/rs-drive/src/util/storage_flags/update.rs b/packages/rs-drive/src/util/storage_flags/update.rs index 98f33de1342..ddde89bc8fa 100644 --- a/packages/rs-drive/src/util/storage_flags/update.rs +++ b/packages/rs-drive/src/util/storage_flags/update.rs @@ -14,7 +14,16 @@ impl StorageFlags { /// A replace merges the old flags into the new ones with `UseTheirs`, so /// new flags naming a different owner transfer the bytes to that owner. /// That holds across kinds: an identity can hand bytes to a contract - /// bucket and a bucket to an identity. + /// bucket and a bucket to an identity, and it holds for every replace + /// shape, including one that moves no bytes (the crate keeps the old + /// owner there; this generation transfers, so that a replace GroveDB + /// first priced as a shrink resolves the same way when it is priced + /// again as same size). + /// + /// GroveDB prices a replace with the old flags attached and re-prices + /// only when this closure reports a change, so a change of header width + /// (35 bytes for an identity owner, 37 for a bucket owner) is reported + /// even when the merged flags already equal the proposed ones. pub fn update_element_flags_typed( cost: &StorageCost, old_flags: Option, @@ -45,7 +54,11 @@ impl StorageFlags { cost.added_bytes, MergingOwnersStrategy::UseTheirs, )?; - Ok(Self::replace_if_changed(combined_storage_flags, new_flags)) + Ok(Self::replace_and_report( + &old_flags, + combined_storage_flags, + new_flags, + )) } OperationStorageTransitionType::OperationUpdateSmallerSize => { let combined_storage_flags = old_storage_flags.combine_removed_bytes( @@ -53,24 +66,39 @@ impl StorageFlags { &cost.removed_bytes, MergingOwnersStrategy::UseTheirs, )?; - Ok(Self::replace_if_changed(combined_storage_flags, new_flags)) + Ok(Self::replace_and_report( + &old_flags, + combined_storage_flags, + new_flags, + )) } OperationStorageTransitionType::OperationUpdateSameSize => { - // a same-size update keeps the old flags - *new_flags = old_storage_flags.to_element_flags(); - Ok(true) + let combined_storage_flags = old_storage_flags + .combine_same_size(new_storage_flags, MergingOwnersStrategy::UseTheirs)?; + Ok(Self::replace_and_report( + &old_flags, + combined_storage_flags, + new_flags, + )) } _ => Ok(false), } } - fn replace_if_changed(combined: StorageFlags, new_flags: &mut ElementFlags) -> bool { + /// Writes the combined flags over the proposed ones and reports whether + /// GroveDB has to price the element again: when the bytes changed, or + /// when the flags kept their bytes but differ in width from the old + /// flags the replace was priced with. + fn replace_and_report( + old_flags: &ElementFlags, + combined: StorageFlags, + new_flags: &mut ElementFlags, + ) -> bool { let combined_flags = combined.to_element_flags(); - if combined_flags == *new_flags { - false - } else { + let changed = combined_flags != *new_flags; + if changed { *new_flags = combined_flags; - true } + changed || old_flags.len() != new_flags.len() } } From 1cb176d7bcf41c927b57111cc92a06b81020ddaf Mon Sep 17 00:00:00 2001 From: DCG-Claude Date: Thu, 17 Sep 2026 16:27:08 -0500 Subject: [PATCH 08/13] fix(drive): keep refund owners out of the plain cost combiner combine_cost_operations folded typed cost operations into a plain OperationCost, which erased the recorded refund owners that operation_cost refuses to drop; a caller rewrapping the aggregate as a CalculatedCostOperation could then have the shipped decoder price a bucket's carrier key as an identity. The plain combiner now folds only plain costs, as it did before this branch, and a new combine_cost_operations_with_refund_owners aggregates both kinds while merging owner maps with collision detection. Also from the review: the typed split callback consumes its decoded flags instead of cloning the epoch map, and FeeRefunds::checked_add_assign refuses an owner record that arrives without credits when the accumulator already holds those credits unowned. Refs #4689 Co-Authored-By: Claude Fable 5.1 --- book/src/drive/cost-tracking.md | 2 +- packages/rs-dpp/src/fee/fee_result/refunds.rs | 29 +++- packages/rs-drive/src/fees/op.rs | 158 ++++++++++++++++-- .../rs-drive/src/util/storage_flags/split.rs | 1 + 4 files changed, 171 insertions(+), 19 deletions(-) diff --git a/book/src/drive/cost-tracking.md b/book/src/drive/cost-tracking.md index 094c2d206a5..4cce6508922 100644 --- a/book/src/drive/cost-tracking.md +++ b/book/src/drive/cost-tracking.md @@ -114,7 +114,7 @@ A fee result that was already computed elsewhere and just needs to be included i ### CalculatedCostOperationWithRefundOwners -A `CalculatedCostOperation` whose sectioned storage removal comes with the typed `RefundOwner` recorded for every carrier key when the removed bytes were split. It is pushed by the batch apply generations that split removed bytes with typed storage flags (`push_drive_operation_result_with_refund_owners`). `operation_cost()` rejects it on purpose: a fee decoder that predates typed owners reaches `operation_cost()` through its catch-all arm and therefore fails closed instead of pricing a refund whose owner it cannot route. `combine_cost_operations` sums its cost like any other. See [Refunds](../fees/overview.md#refunds) for how the owners are recorded. +A `CalculatedCostOperation` whose sectioned storage removal comes with the typed `RefundOwner` recorded for every carrier key when the removed bytes were split. It is pushed by the batch apply generations that split removed bytes with typed storage flags (`push_drive_operation_result_with_refund_owners`). `operation_cost()` rejects it on purpose: a fee decoder that predates typed owners reaches `operation_cost()` through its catch-all arm and therefore fails closed instead of pricing a refund whose owner it cannot route. `combine_cost_operations` leaves it out for the same reason (folding it into a plain `OperationCost` would erase the owners); `combine_cost_operations_with_refund_owners` sums plain and typed costs together and returns the merged owner map, refusing one carrier key recorded for two owners. See [Refunds](../fees/overview.md#refunds) for how the owners are recorded. ## BaseOp: Arithmetic Operation Costs diff --git a/packages/rs-dpp/src/fee/fee_result/refunds.rs b/packages/rs-dpp/src/fee/fee_result/refunds.rs index 6fa5c60d0cb..e34593c35c3 100644 --- a/packages/rs-dpp/src/fee/fee_result/refunds.rs +++ b/packages/rs-dpp/src/fee/fee_result/refunds.rs @@ -210,8 +210,8 @@ impl FeeRefunds { } } for (identifier, owner) in &rhs_owners { - if let Some(existing) = self.1.get(identifier) { - if existing != owner { + match self.1.get(identifier) { + Some(existing) if existing != owner => { return Err(ProtocolError::CorruptedCodeExecution(format!( "storage removal carrier key {} is recorded for two different refund owners: {:?} and {:?}", hex::encode(identifier), @@ -219,6 +219,15 @@ impl FeeRefunds { owner ))); } + // an owner record arriving without credits must not lend an + // owner to credits already held without one + None if self.0.contains_key(identifier) => { + return Err(ProtocolError::CorruptedCodeExecution(format!( + "storage refund carrier key {} has no recorded refund owner", + hex::encode(identifier) + ))); + } + _ => {} } } for (identifier, mut int_map_b) in rhs_credits.into_iter() { @@ -634,10 +643,24 @@ mod tests { let mut right = owned; let before = right.clone(); assert!(matches!( - right.checked_add_assign(unowned), + right.checked_add_assign(unowned.clone()), Err(ProtocolError::CorruptedCodeExecution(_)) )); assert_eq!(right, before, "a rejected merge changes nothing"); + + // an owner record with no credits of its own must not attach + // itself to credits held without an owner + let owner_only = FeeRefunds( + CreditsPerEpochByIdentifier::new(), + RefundOwnersByIdentifier::from_iter([(key, bucket)]), + ); + let mut left = unowned; + let before = left.clone(); + assert!(matches!( + left.checked_add_assign(owner_only), + Err(ProtocolError::CorruptedCodeExecution(_)) + )); + assert_eq!(left, before, "a rejected merge changes nothing"); } } diff --git a/packages/rs-drive/src/fees/op.rs b/packages/rs-drive/src/fees/op.rs index e701689ad51..b7d267e6fde 100644 --- a/packages/rs-drive/src/fees/op.rs +++ b/packages/rs-drive/src/fees/op.rs @@ -533,20 +533,64 @@ impl LowLevelDriveOperation { .any(|operation| matches!(operation, RepaidIdentityDebt(_))) } - /// Filters the groveDB ops from a list of operations and puts them in a `GroveDbOpBatch`. + /// Sums the plain calculated costs of a list of operations. + /// + /// Only `CalculatedCostOperation` is folded. A typed cost operation is + /// left out on purpose, as function operations and pre-calculated fee + /// results are: folding it into a plain `OperationCost` would erase the + /// recorded refund owners that `operation_cost` refuses to drop, and the + /// operation itself stays in the list for the fee decoder. Use + /// [`Self::combine_cost_operations_with_refund_owners`] to aggregate + /// typed costs without losing their owners. pub fn combine_cost_operations(operations: &[LowLevelDriveOperation]) -> OperationCost { let mut cost = OperationCost::default(); - operations.iter().for_each(|op| match op { - CalculatedCostOperation(operation_cost) - | CalculatedCostOperationWithRefundOwners { - cost: operation_cost, - .. - } => cost += operation_cost.clone(), - _ => {} + operations.iter().for_each(|op| { + if let CalculatedCostOperation(operation_cost) = op { + cost += operation_cost.clone() + } }); cost } + /// Sums the plain and typed calculated costs of a list of operations, + /// keeping the recorded refund owners of the typed ones. + /// + /// Owner maps merge by carrier key; one key recorded for two different + /// owners is a collision and is reported, never resolved by picking one. + pub fn combine_cost_operations_with_refund_owners( + operations: &[LowLevelDriveOperation], + ) -> Result<(OperationCost, RefundOwnersByIdentifier), Error> { + let mut cost = OperationCost::default(); + let mut owners = RefundOwnersByIdentifier::new(); + for op in operations { + match op { + CalculatedCostOperation(operation_cost) => cost += operation_cost.clone(), + CalculatedCostOperationWithRefundOwners { + cost: operation_cost, + refund_owners, + } => { + for (key, owner) in refund_owners { + match owners.get(key) { + Some(existing) if existing != owner => { + return Err(Error::Drive(DriveError::CorruptedCodeExecution( + "two different refund owners share one storage removal \ + carrier key across combined cost operations", + ))); + } + Some(_) => {} + None => { + owners.insert(*key, *owner); + } + } + } + cost += operation_cost.clone(); + } + _ => {} + } + } + Ok((cost, owners)) + } + /// Filters the groveDB ops from a list of operations and puts them in a `GroveDbOpBatch`. pub fn grovedb_operations_batch( insert_operations: &[LowLevelDriveOperation], @@ -2422,8 +2466,11 @@ mod tests { )); } + /// The plain combiner cannot carry owners, so it leaves typed costs out + /// rather than laundering them into a plain cost that the shipped + /// decoder would price by reading a bucket's carrier key as an identity. #[test] - fn should_sum_typed_cost_operations_into_combined_costs() { + fn should_leave_typed_cost_operations_out_of_the_plain_combiner() { let cost = OperationCost { seek_count: 4, storage_cost: StorageCost { @@ -2435,21 +2482,102 @@ mod tests { hash_node_calls: 2, sinsemilla_hash_calls: 0, }; + let owner = RefundOwner::Identity(Identifier::from([5u8; 32])); let operations = vec![ CalculatedCostOperation(cost.clone()), CalculatedCostOperationWithRefundOwners { cost: cost.clone(), - refund_owners: Default::default(), + refund_owners: BTreeMap::from([(owner.removal_key(), owner)]), }, ]; let combined = LowLevelDriveOperation::combine_cost_operations(&operations); - assert_eq!(combined.seek_count, 8); - assert_eq!(combined.storage_cost.added_bytes, 14); - assert_eq!(combined.storage_cost.replaced_bytes, 6); - assert_eq!(combined.storage_loaded_bytes, 22); - assert_eq!(combined.hash_node_calls, 4); + assert_eq!(combined, cost, "only the plain cost is folded"); + } + + #[test] + fn should_sum_typed_cost_operations_and_merge_their_owners() { + let cost = OperationCost { + seek_count: 4, + storage_cost: StorageCost { + added_bytes: 7, + replaced_bytes: 3, + removed_bytes: StorageRemovedBytes::NoStorageRemoval, + }, + storage_loaded_bytes: 11, + hash_node_calls: 2, + sinsemilla_hash_calls: 0, + }; + let identity = RefundOwner::Identity(Identifier::from([5u8; 32])); + let bucket = RefundOwner::ContractBucket { + contract_id: Identifier::from([6u8; 32]), + position: 1, + }; + let operations = vec![ + CalculatedCostOperation(cost.clone()), + CalculatedCostOperationWithRefundOwners { + cost: cost.clone(), + refund_owners: BTreeMap::from([(identity.removal_key(), identity)]), + }, + CalculatedCostOperationWithRefundOwners { + cost: cost.clone(), + refund_owners: BTreeMap::from([ + (identity.removal_key(), identity), + (bucket.removal_key(), bucket), + ]), + }, + FunctionOperation(FunctionOp::new_with_round_count(HashFunction::Sha256, 1)), + ]; + + let (combined, owners) = + LowLevelDriveOperation::combine_cost_operations_with_refund_owners(&operations) + .expect("should combine"); + + assert_eq!(combined.seek_count, 12); + assert_eq!(combined.storage_cost.added_bytes, 21); + assert_eq!(combined.storage_cost.replaced_bytes, 9); + assert_eq!(combined.storage_loaded_bytes, 33); + assert_eq!(combined.hash_node_calls, 6); + assert_eq!( + owners, + BTreeMap::from([ + (identity.removal_key(), identity), + (bucket.removal_key(), bucket), + ]) + ); + } + + #[test] + fn should_reject_one_carrier_key_with_two_owners_across_combined_cost_operations() { + let key = [9u8; 32]; + let operations = vec![ + CalculatedCostOperationWithRefundOwners { + cost: OperationCost::default(), + refund_owners: BTreeMap::from([( + key, + RefundOwner::Identity(Identifier::from(key)), + )]), + }, + CalculatedCostOperationWithRefundOwners { + cost: OperationCost::default(), + refund_owners: BTreeMap::from([( + key, + RefundOwner::ContractBucket { + contract_id: Identifier::from([1u8; 32]), + position: 0, + }, + )]), + }, + ]; + + let result = + LowLevelDriveOperation::combine_cost_operations_with_refund_owners(&operations); + + assert!(matches!( + result, + Err(Error::Drive(DriveError::CorruptedCodeExecution(_))) + )); } // --------------------------------------------------------------- diff --git a/packages/rs-drive/src/util/storage_flags/split.rs b/packages/rs-drive/src/util/storage_flags/split.rs index 05006ee2999..9c5b627fd83 100644 --- a/packages/rs-drive/src/util/storage_flags/split.rs +++ b/packages/rs-drive/src/util/storage_flags/split.rs @@ -51,6 +51,7 @@ impl StorageFlags { Self::record_refund_owner(owner, refund_owners)?; } Ok(storage_flags + .into_crate_flags_keyed_by_removal_key() .split_storage_removed_bytes(removed_key_bytes, removed_value_bytes)) } } From 8ede1803de91c63db1d952fd8899c7dc317c32b8 Mon Sep 17 00:00:00 2001 From: DCG-Claude Date: Thu, 17 Sep 2026 17:55:30 -0500 Subject: [PATCH 09/13] fix(drive): attribute every sectioned key when aggregating typed costs combine_cost_operations_with_refund_owners merged owner maps without looking at the removals they described. A plain cost's sectioned identity keys were left unattributed, so an aggregate of a plain identity removal and a typed bucket removal could not be priced by from_typed_storage_removal, and a plain key equal to a bucket carrier key was silently attributed to the bucket. A typed input could likewise contribute bytes under a key it never recorded and have another input's owner cover them. The aggregate now establishes the owner of every sectioned key per input before anything merges: a plain cost's keys are identities, as its identity only flags say; a typed cost must have recorded an owner for each of its own keys or the aggregate fails; the system key is never an owner; one key attributed to two owners within or across inputs is refused. Tests use real sectioned removals, including pricing the aggregate through the typed constructor. Refs #4689 Co-Authored-By: Claude Fable 5.1 --- book/src/drive/cost-tracking.md | 2 +- packages/rs-drive/src/fees/op.rs | 218 +++++++++++++++++++++++-------- 2 files changed, 166 insertions(+), 54 deletions(-) diff --git a/book/src/drive/cost-tracking.md b/book/src/drive/cost-tracking.md index 4cce6508922..50872290464 100644 --- a/book/src/drive/cost-tracking.md +++ b/book/src/drive/cost-tracking.md @@ -114,7 +114,7 @@ A fee result that was already computed elsewhere and just needs to be included i ### CalculatedCostOperationWithRefundOwners -A `CalculatedCostOperation` whose sectioned storage removal comes with the typed `RefundOwner` recorded for every carrier key when the removed bytes were split. It is pushed by the batch apply generations that split removed bytes with typed storage flags (`push_drive_operation_result_with_refund_owners`). `operation_cost()` rejects it on purpose: a fee decoder that predates typed owners reaches `operation_cost()` through its catch-all arm and therefore fails closed instead of pricing a refund whose owner it cannot route. `combine_cost_operations` leaves it out for the same reason (folding it into a plain `OperationCost` would erase the owners); `combine_cost_operations_with_refund_owners` sums plain and typed costs together and returns the merged owner map, refusing one carrier key recorded for two owners. See [Refunds](../fees/overview.md#refunds) for how the owners are recorded. +A `CalculatedCostOperation` whose sectioned storage removal comes with the typed `RefundOwner` recorded for every carrier key when the removed bytes were split. It is pushed by the batch apply generations that split removed bytes with typed storage flags (`push_drive_operation_result_with_refund_owners`). `operation_cost()` rejects it on purpose: a fee decoder that predates typed owners reaches `operation_cost()` through its catch-all arm and therefore fails closed instead of pricing a refund whose owner it cannot route. `combine_cost_operations` leaves it out for the same reason (folding it into a plain `OperationCost` would erase the owners); `combine_cost_operations_with_refund_owners` sums plain and typed costs together and returns the owner of every sectioned removal key: a plain cost's keys are identities, a typed cost must have recorded an owner for each of its own keys, the system key is never an owner, and one key attributed to two owners is refused. See [Refunds](../fees/overview.md#refunds) for how the owners are recorded. ## BaseOp: Arithmetic Operation Costs diff --git a/packages/rs-drive/src/fees/op.rs b/packages/rs-drive/src/fees/op.rs index b7d267e6fde..83db468947e 100644 --- a/packages/rs-drive/src/fees/op.rs +++ b/packages/rs-drive/src/fees/op.rs @@ -32,7 +32,9 @@ use dpp::block::epoch::Epoch; use dpp::fee::default_costs::CachedEpochIndexFeeVersions; use dpp::fee::fee_result::refunds::{FeeRefunds, RefundOwnersByIdentifier}; use dpp::fee::fee_result::{FeeResult, LifetimeStorageFees}; +use dpp::fee::refund_owner::{RefundOwner, SYSTEM_REFUND_CARRIER_KEY}; use dpp::fee::Credits; +use dpp::identifier::Identifier as PlatformIdentifier; use platform_version::version::fee::FeeVersion; /// Base ops @@ -553,10 +555,16 @@ impl LowLevelDriveOperation { } /// Sums the plain and typed calculated costs of a list of operations, - /// keeping the recorded refund owners of the typed ones. + /// establishing the refund owner of every sectioned removal key. /// - /// Owner maps merge by carrier key; one key recorded for two different - /// owners is a collision and is reported, never resolved by picking one. + /// Each input attributes its own removal keys before anything merges: a + /// plain cost's sectioned keys are identities, as the identity-only flags + /// that produced them say; a typed cost must carry a recorded owner for + /// every sectioned key of its own, and an unrecorded key is an invariant + /// failure rather than something to guess at. The system key is never an + /// owner. Owners then merge by carrier key, and one key attributed to two + /// different owners, within an input or across inputs, is reported and + /// never resolved by picking one. pub fn combine_cost_operations_with_refund_owners( operations: &[LowLevelDriveOperation], ) -> Result<(OperationCost, RefundOwnersByIdentifier), Error> { @@ -564,25 +572,46 @@ impl LowLevelDriveOperation { let mut owners = RefundOwnersByIdentifier::new(); for op in operations { match op { - CalculatedCostOperation(operation_cost) => cost += operation_cost.clone(), + CalculatedCostOperation(operation_cost) => { + if let SectionedStorageRemoval(removal) = + &operation_cost.storage_cost.removed_bytes + { + for key in removal.keys() { + if *key == SYSTEM_REFUND_CARRIER_KEY { + continue; + } + Self::record_refund_owner( + &mut owners, + *key, + RefundOwner::Identity(PlatformIdentifier::from(*key)), + )?; + } + } + cost += operation_cost.clone(); + } CalculatedCostOperationWithRefundOwners { cost: operation_cost, refund_owners, } => { - for (key, owner) in refund_owners { - match owners.get(key) { - Some(existing) if existing != owner => { - return Err(Error::Drive(DriveError::CorruptedCodeExecution( - "two different refund owners share one storage removal \ - carrier key across combined cost operations", - ))); - } - Some(_) => {} - None => { - owners.insert(*key, *owner); + if let SectionedStorageRemoval(removal) = + &operation_cost.storage_cost.removed_bytes + { + for key in removal.keys() { + if *key == SYSTEM_REFUND_CARRIER_KEY { + continue; } + let owner = refund_owners.get(key).ok_or(Error::Drive( + DriveError::CorruptedCodeExecution( + "a typed cost operation sections removed bytes under a \ + carrier key it recorded no refund owner for", + ), + ))?; + Self::record_refund_owner(&mut owners, *key, *owner)?; } } + for (key, owner) in refund_owners { + Self::record_refund_owner(&mut owners, *key, *owner)?; + } cost += operation_cost.clone(); } _ => {} @@ -591,6 +620,26 @@ impl LowLevelDriveOperation { Ok((cost, owners)) } + fn record_refund_owner( + owners: &mut RefundOwnersByIdentifier, + key: [u8; 32], + owner: RefundOwner, + ) -> Result<(), Error> { + match owners.get(&key) { + Some(existing) if *existing != owner => { + Err(Error::Drive(DriveError::CorruptedCodeExecution( + "two different refund owners share one storage removal carrier key across \ + combined cost operations", + ))) + } + Some(_) => Ok(()), + None => { + owners.insert(key, owner); + Ok(()) + } + } + } + /// Filters the groveDB ops from a list of operations and puts them in a `GroveDbOpBatch`. pub fn grovedb_operations_batch( insert_operations: &[LowLevelDriveOperation], @@ -2034,7 +2083,6 @@ impl DriveCost for OperationCost { #[allow(clippy::identity_op)] mod tests { use super::*; - use dpp::fee::refund_owner::RefundOwner; use dpp::identifier::Identifier; use grovedb_costs::storage_cost::removal::{ StorageRemovalPerEpochByIdentifier, StorageRemovedBytes, @@ -2496,36 +2544,45 @@ mod tests { assert_eq!(combined, cost, "only the plain cost is folded"); } - #[test] - fn should_sum_typed_cost_operations_and_merge_their_owners() { - let cost = OperationCost { - seek_count: 4, + fn sectioned_cost(entries: &[([u8; 32], u32)]) -> OperationCost { + let mut removal = StorageRemovalPerEpochByIdentifier::new(); + for (key, bytes) in entries { + removal.insert(*key, IntMap::from_iter([(0u16, *bytes)])); + } + OperationCost { + seek_count: 1, storage_cost: StorageCost { - added_bytes: 7, - replaced_bytes: 3, - removed_bytes: StorageRemovedBytes::NoStorageRemoval, + added_bytes: 0, + replaced_bytes: 0, + removed_bytes: SectionedStorageRemoval(removal), }, - storage_loaded_bytes: 11, - hash_node_calls: 2, + storage_loaded_bytes: 0, + hash_node_calls: 0, sinsemilla_hash_calls: 0, - }; + } + } + + #[test] + fn should_attribute_every_sectioned_key_when_combining_plain_and_typed_costs() { let identity = RefundOwner::Identity(Identifier::from([5u8; 32])); let bucket = RefundOwner::ContractBucket { contract_id: Identifier::from([6u8; 32]), position: 1, }; let operations = vec![ - CalculatedCostOperation(cost.clone()), + // a plain cost from identity-only flags: its keys are identities + CalculatedCostOperation(sectioned_cost(&[ + (identity.removal_key(), 100), + (SYSTEM_REFUND_CARRIER_KEY, 7), + ])), CalculatedCostOperationWithRefundOwners { - cost: cost.clone(), - refund_owners: BTreeMap::from([(identity.removal_key(), identity)]), + cost: sectioned_cost(&[(bucket.removal_key(), 40), (SYSTEM_REFUND_CARRIER_KEY, 3)]), + refund_owners: BTreeMap::from([(bucket.removal_key(), bucket)]), }, + // the same identity again through a typed cost merges cleanly CalculatedCostOperationWithRefundOwners { - cost: cost.clone(), - refund_owners: BTreeMap::from([ - (identity.removal_key(), identity), - (bucket.removal_key(), bucket), - ]), + cost: sectioned_cost(&[(identity.removal_key(), 20)]), + refund_owners: BTreeMap::from([(identity.removal_key(), identity)]), }, FunctionOperation(FunctionOp::new_with_round_count(HashFunction::Sha256, 1)), ]; @@ -2534,11 +2591,13 @@ mod tests { LowLevelDriveOperation::combine_cost_operations_with_refund_owners(&operations) .expect("should combine"); - assert_eq!(combined.seek_count, 12); - assert_eq!(combined.storage_cost.added_bytes, 21); - assert_eq!(combined.storage_cost.replaced_bytes, 9); - assert_eq!(combined.storage_loaded_bytes, 33); - assert_eq!(combined.hash_node_calls, 6); + assert_eq!(combined.seek_count, 3); + let SectionedStorageRemoval(removal) = &combined.storage_cost.removed_bytes else { + panic!("sectioned removals stay sectioned"); + }; + assert_eq!(removal[&identity.removal_key()].get(0u16), Some(&120)); + assert_eq!(removal[&bucket.removal_key()].get(0u16), Some(&40)); + assert_eq!(removal[&SYSTEM_REFUND_CARRIER_KEY].get(0u16), Some(&10)); assert_eq!( owners, BTreeMap::from([ @@ -2546,12 +2605,75 @@ mod tests { (bucket.removal_key(), bucket), ]) ); + assert!( + !owners.contains_key(&SYSTEM_REFUND_CARRIER_KEY), + "the system key is never an owner" + ); + + // the aggregate prices as a whole through the typed constructor + let mut priced_removal = removal.clone(); + priced_removal.remove(&SYSTEM_REFUND_CARRIER_KEY); + FeeRefunds::from_typed_storage_removal( + priced_removal, + &owners, + 3, + 20, + &BTreeMap::from([(0, FeeVersion::first())]), + ) + .expect("every key has an owner"); } #[test] - fn should_reject_one_carrier_key_with_two_owners_across_combined_cost_operations() { - let key = [9u8; 32]; + fn should_reject_a_typed_cost_that_sections_bytes_under_an_unrecorded_key() { + let bucket = RefundOwner::ContractBucket { + contract_id: Identifier::from([6u8; 32]), + position: 1, + }; + // 100 unattributed bytes plus 40 attributed bytes under one key must + // not become 140 bucket owned bytes let operations = vec![ + CalculatedCostOperationWithRefundOwners { + cost: sectioned_cost(&[(bucket.removal_key(), 100)]), + refund_owners: BTreeMap::new(), + }, + CalculatedCostOperationWithRefundOwners { + cost: sectioned_cost(&[(bucket.removal_key(), 40)]), + refund_owners: BTreeMap::from([(bucket.removal_key(), bucket)]), + }, + ]; + + let result = + LowLevelDriveOperation::combine_cost_operations_with_refund_owners(&operations); + + assert!(matches!( + result, + Err(Error::Drive(DriveError::CorruptedCodeExecution(_))) + )); + } + + #[test] + fn should_reject_one_carrier_key_with_two_owners_across_combined_cost_operations() { + let bucket = RefundOwner::ContractBucket { + contract_id: Identifier::from([1u8; 32]), + position: 0, + }; + let key = bucket.removal_key(); + + // a plain identity removal whose key equals the bucket carrier key + let plain_then_typed = vec![ + CalculatedCostOperation(sectioned_cost(&[(key, 10)])), + CalculatedCostOperationWithRefundOwners { + cost: sectioned_cost(&[(key, 1)]), + refund_owners: BTreeMap::from([(key, bucket)]), + }, + ]; + assert!(matches!( + LowLevelDriveOperation::combine_cost_operations_with_refund_owners(&plain_then_typed), + Err(Error::Drive(DriveError::CorruptedCodeExecution(_))) + )); + + // two typed records naming different owners for one key + let typed_conflict = vec![ CalculatedCostOperationWithRefundOwners { cost: OperationCost::default(), refund_owners: BTreeMap::from([( @@ -2561,21 +2683,11 @@ mod tests { }, CalculatedCostOperationWithRefundOwners { cost: OperationCost::default(), - refund_owners: BTreeMap::from([( - key, - RefundOwner::ContractBucket { - contract_id: Identifier::from([1u8; 32]), - position: 0, - }, - )]), + refund_owners: BTreeMap::from([(key, bucket)]), }, ]; - - let result = - LowLevelDriveOperation::combine_cost_operations_with_refund_owners(&operations); - assert!(matches!( - result, + LowLevelDriveOperation::combine_cost_operations_with_refund_owners(&typed_conflict), Err(Error::Drive(DriveError::CorruptedCodeExecution(_))) )); } From 7e130e4eea94908f11aa632811f3e828ef12ee72 Mon Sep 17 00:00:00 2001 From: DCG-Claude Date: Fri, 18 Sep 2026 04:35:26 -0500 Subject: [PATCH 10/13] fix(drive): skip the system key when merging typed owner maps The typed cost aggregate skipped the system carrier key when attributing sectioned removals but not when merging an input's recorded owner map, so a hand-built operation could hand a consumer a system-key owner that is never routable. The merge loop now skips it like the two loops above it; a test pins that a system-key entry in an owner map records nothing. Refs #4689 Co-Authored-By: Claude Fable 5.1 --- packages/rs-drive/src/fees/op.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/packages/rs-drive/src/fees/op.rs b/packages/rs-drive/src/fees/op.rs index 83db468947e..ee675f0c31e 100644 --- a/packages/rs-drive/src/fees/op.rs +++ b/packages/rs-drive/src/fees/op.rs @@ -610,6 +610,9 @@ impl LowLevelDriveOperation { } } for (key, owner) in refund_owners { + if *key == SYSTEM_REFUND_CARRIER_KEY { + continue; + } Self::record_refund_owner(&mut owners, *key, *owner)?; } cost += operation_cost.clone(); @@ -2623,6 +2626,28 @@ mod tests { .expect("every key has an owner"); } + /// The typed split closure never records the system key (the all-zero + /// identity records nothing, a bucket deriving it fails the batch), so a + /// system key in an owner map can only come from a hand-built operation. + /// The aggregate drops it rather than hand a consumer an owner that is + /// not routable. + #[test] + fn should_never_record_the_system_key_as_an_owner_from_a_typed_owner_map() { + let operations = vec![CalculatedCostOperationWithRefundOwners { + cost: sectioned_cost(&[(SYSTEM_REFUND_CARRIER_KEY, 7)]), + refund_owners: BTreeMap::from([( + SYSTEM_REFUND_CARRIER_KEY, + RefundOwner::Identity(Identifier::from(SYSTEM_REFUND_CARRIER_KEY)), + )]), + }]; + + let (_, owners) = + LowLevelDriveOperation::combine_cost_operations_with_refund_owners(&operations) + .expect("should combine"); + + assert!(owners.is_empty()); + } + #[test] fn should_reject_a_typed_cost_that_sections_bytes_under_an_unrecorded_key() { let bucket = RefundOwner::ContractBucket { From 19cd7fa152666072210e29ca09b3b14ce15500dd Mon Sep 17 00:00:00 2001 From: DCG-Claude Date: Tue, 29 Sep 2026 20:07:17 -0500 Subject: [PATCH 11/13] fix(dpp): reject refund owners recorded under another owner's carrier key The typed refund constructor checked only that a carrier key had an owner record, not that the recorded owner derives that key, and the fields of FeeRefunds are public. A mismatched record would route a refund to the wrong owner. from_typed_storage_removal and iter_typed now require owner.removal_key() to equal the carrier key, so every typed accessor fails closed on such a record. Co-Authored-By: Claude Opus 5.5 --- packages/rs-dpp/src/fee/fee_result/mod.rs | 5 +- packages/rs-dpp/src/fee/fee_result/refunds.rs | 111 ++++++++++++++---- 2 files changed, 93 insertions(+), 23 deletions(-) diff --git a/packages/rs-dpp/src/fee/fee_result/mod.rs b/packages/rs-dpp/src/fee/fee_result/mod.rs index 050a67d1f65..e0a342dcb03 100644 --- a/packages/rs-dpp/src/fee/fee_result/mod.rs +++ b/packages/rs-dpp/src/fee/fee_result/mod.rs @@ -146,8 +146,9 @@ impl BalanceChangeForIdentity { /// Returns the refund amount of credits for every recorded owner other /// than the paying identity, keyed by the typed owner. /// - /// A carrier key without a recorded owner is an error: the refund cannot - /// be routed and must not be guessed at. + /// A carrier key without a recorded owner, or recorded for an owner whose + /// carrier key differs, is an error: the refund cannot be routed and must + /// not be guessed at. pub fn other_typed_refunds(&self) -> Result, ProtocolError> { self.fee_result .fee_refunds diff --git a/packages/rs-dpp/src/fee/fee_result/refunds.rs b/packages/rs-dpp/src/fee/fee_result/refunds.rs index e34593c35c3..ecf49dd1234 100644 --- a/packages/rs-dpp/src/fee/fee_result/refunds.rs +++ b/packages/rs-dpp/src/fee/fee_result/refunds.rs @@ -117,21 +117,36 @@ impl FeeRefunds { let recorded_owners = refunds_per_epoch_by_identifier .keys() .map(|identifier| { - owners - .get(identifier) - .map(|owner| (*identifier, *owner)) - .ok_or_else(|| { - ProtocolError::CorruptedCodeExecution(format!( - "storage removal carrier key {} has no recorded refund owner", - hex::encode(identifier) - )) - }) + let owner = owners.get(identifier).ok_or_else(|| { + ProtocolError::CorruptedCodeExecution(format!( + "storage removal carrier key {} has no recorded refund owner", + hex::encode(identifier) + )) + })?; + Self::ensure_owner_matches_key(identifier, owner)?; + Ok((*identifier, *owner)) }) .collect::>()?; Ok(Self(refunds_per_epoch_by_identifier, recorded_owners)) } + /// A recorded owner must be the owner its carrier key was derived from; + /// any other record would route the refund to the wrong owner. + fn ensure_owner_matches_key( + identifier: &[u8; 32], + owner: &RefundOwner, + ) -> Result<(), ProtocolError> { + if owner.removal_key() != *identifier { + return Err(ProtocolError::CorruptedCodeExecution(format!( + "storage refund carrier key {} is recorded for {:?}, whose carrier key differs", + hex::encode(identifier), + owner + ))); + } + Ok(()) + } + fn price_storage_removal( storage_removal: I, current_epoch_index: EpochIndex, @@ -273,22 +288,23 @@ impl FeeRefunds { /// Iterates the refunds with their recorded owners. /// - /// A carrier key without a recorded owner yields an error: a refund that - /// cannot name its owner cannot be routed and must halt rather than be - /// burned, minted or guessed. + /// A carrier key without a recorded owner, or recorded for an owner whose + /// carrier key is a different one, yields an error: a refund that cannot + /// name its owner cannot be routed and must halt rather than be burned, + /// minted or guessed. pub fn iter_typed( &self, ) -> impl Iterator> { self.0.iter().map(|(identifier, credits_per_epoch)| { - self.owner_of(identifier) - .map(|owner| (owner, identifier, credits_per_epoch)) - .ok_or_else(|| { - ProtocolError::CorruptedCodeExecution(format!( - "storage refund carrier key {} has no recorded refund owner", - hex::encode(identifier) - )) - }) + let owner = self.owner_of(identifier).ok_or_else(|| { + ProtocolError::CorruptedCodeExecution(format!( + "storage refund carrier key {} has no recorded refund owner", + hex::encode(identifier) + )) + })?; + Self::ensure_owner_matches_key(identifier, &owner)?; + Ok((owner, identifier, credits_per_epoch)) }) } @@ -363,7 +379,8 @@ impl FeeRefunds { /// /// Owners are compared as typed values, so an identity and a contract /// bucket never match each other. A carrier key without a recorded owner - /// is an error, as is a sum that overflows. + /// or recorded for an owner whose carrier key differs is an error, as is + /// a sum that overflows. pub fn calculate_all_refunds_except_owner( &self, skip_owner: &RefundOwner, @@ -548,6 +565,30 @@ mod tests { Err(ProtocolError::CorruptedCodeExecution(_)) )); } + + #[test] + fn should_reject_an_owner_recorded_under_another_owners_carrier_key() { + let bucket = bucket_owner(6, 2); + let identity_key = [5u8; 32]; + let owners = RefundOwnersByIdentifier::from_iter([(identity_key, bucket)]); + let storage_removal = BytesPerEpochByIdentifier::from_iter([( + identity_key, + IntMap::from_iter([(1, 200)]), + )]); + + let result = FeeRefunds::from_typed_storage_removal( + storage_removal, + &owners, + 3, + 20, + &EPOCH_CHANGE_FEE_VERSION_TEST, + ); + + assert!(matches!( + result, + Err(ProtocolError::CorruptedCodeExecution(_)) + )); + } } mod checked_add_assign { @@ -727,6 +768,34 @@ mod tests { )); } + #[test] + fn should_fail_closed_on_an_owner_recorded_under_another_carrier_key() { + let bucket = bucket_owner(3, 4); + let identity = RefundOwner::Identity(Identifier::from([4u8; 32])); + let refunds = FeeRefunds( + CreditsPerEpochByIdentifier::from_iter([( + identity.removal_key(), + CreditsPerEpoch::from_iter([(0, 100)]), + )]), + RefundOwnersByIdentifier::from_iter([(identity.removal_key(), bucket)]), + ); + + let entries: Vec<_> = refunds.iter_typed().collect(); + assert_eq!(entries.len(), 1); + assert!(matches!( + entries[0], + Err(ProtocolError::CorruptedCodeExecution(_)) + )); + assert!(matches!( + refunds.calculate_all_refunds_except_owner(&identity), + Err(ProtocolError::CorruptedCodeExecution(_)) + )); + assert!(matches!( + refunds.ensure_identity_owners_only(), + Err(ProtocolError::CorruptedCodeExecution(_)) + )); + } + #[test] fn should_guard_the_identity_keyed_view_against_bucket_owned_refunds() { let identity = RefundOwner::Identity(Identifier::from([1u8; 32])); From af04133c202624b5f7de6d33feac7bb2bf7282d5 Mon Sep 17 00:00:00 2001 From: DCG-Claude Date: Tue, 29 Sep 2026 22:53:43 -0500 Subject: [PATCH 12/13] fix(drive): keep shipped batch move generations on the historical flags type The batch move and group action generation 0 code had been switched to Drive's storage flags enum, which also carries the contract bucket variants. A caller could then hand a shipped move a bucket-owned flag that it would persist without any recorded refund owner. Every caller passes no flags, so the switch bought nothing; these files go back to importing the crate type exactly as the base does. Co-Authored-By: Claude Opus 5.5 --- .../rs-drive/src/drive/group/insert/add_group_action/v0/mod.rs | 2 +- packages/rs-drive/src/util/grove_operations/batch_move/mod.rs | 2 +- .../rs-drive/src/util/grove_operations/batch_move/v0/mod.rs | 2 +- .../util/grove_operations/batch_move_items_in_path_query/mod.rs | 2 +- .../grove_operations/batch_move_items_in_path_query/v0/mod.rs | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/rs-drive/src/drive/group/insert/add_group_action/v0/mod.rs b/packages/rs-drive/src/drive/group/insert/add_group_action/v0/mod.rs index d0a8bf8ac70..12ccb98337c 100644 --- a/packages/rs-drive/src/drive/group/insert/add_group_action/v0/mod.rs +++ b/packages/rs-drive/src/drive/group/insert/add_group_action/v0/mod.rs @@ -13,7 +13,6 @@ use crate::util::grove_operations::{ }; use crate::util::object_size_info::PathKeyInfo::PathFixedSizeKeyRef; use crate::util::object_size_info::{DriveKeyInfo, PathKeyElementInfo}; -use crate::util::storage_flags::StorageFlags; use dpp::block::block_info::BlockInfo; use dpp::data_contract::group::GroupMemberPower; use dpp::data_contract::GroupContractPosition; @@ -27,6 +26,7 @@ use grovedb::batch::KeyInfoPath; use grovedb::element::SumValue; use grovedb::MaybeTree::NotTree; use grovedb::{Element, EstimatedLayerInformation, MaybeTree, TransactionArg, TreeType}; +use grovedb_epoch_based_storage_flags::StorageFlags; use std::collections::HashMap; impl Drive { diff --git a/packages/rs-drive/src/util/grove_operations/batch_move/mod.rs b/packages/rs-drive/src/util/grove_operations/batch_move/mod.rs index e6d234cf090..eb8b64d8214 100644 --- a/packages/rs-drive/src/util/grove_operations/batch_move/mod.rs +++ b/packages/rs-drive/src/util/grove_operations/batch_move/mod.rs @@ -5,9 +5,9 @@ use crate::error::drive::DriveError; use crate::error::Error; use crate::fees::op::LowLevelDriveOperation; use crate::util::grove_operations::BatchMoveApplyType; -use crate::util::storage_flags::StorageFlags; use dpp::version::drive_versions::DriveVersion; use grovedb::TransactionArg; +use grovedb_epoch_based_storage_flags::StorageFlags; use grovedb_path::SubtreePath; impl Drive { diff --git a/packages/rs-drive/src/util/grove_operations/batch_move/v0/mod.rs b/packages/rs-drive/src/util/grove_operations/batch_move/v0/mod.rs index 088630edba8..ff535afa818 100644 --- a/packages/rs-drive/src/util/grove_operations/batch_move/v0/mod.rs +++ b/packages/rs-drive/src/util/grove_operations/batch_move/v0/mod.rs @@ -5,12 +5,12 @@ use crate::fees::op::LowLevelDriveOperation; use crate::fees::op::LowLevelDriveOperation::GroveOperation; use crate::util::grove_operations::pending_grove_operations::pending_grove_operations; use crate::util::grove_operations::{push_drive_operation_result, BatchMoveApplyType, QueryType}; -use crate::util::storage_flags::StorageFlags; use grovedb::batch::key_info::KeyInfo; use grovedb::batch::{KeyInfoPath, QualifiedGroveDbOp}; use grovedb::operations::delete::DeleteOptions; use grovedb::BackwardsReferences; use grovedb::{Element, GroveDb, TransactionArg}; +use grovedb_epoch_based_storage_flags::StorageFlags; use grovedb_path::SubtreePath; use grovedb_storage::rocksdb_storage::RocksDbStorage; use platform_version::version::drive_versions::DriveVersion; diff --git a/packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/mod.rs b/packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/mod.rs index f65f9ad6a8f..7c31d0b7a66 100644 --- a/packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/mod.rs +++ b/packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/mod.rs @@ -8,8 +8,8 @@ use crate::util::grove_operations::BatchMoveApplyType; use dpp::version::drive_versions::DriveVersion; -use crate::util::storage_flags::StorageFlags; use grovedb::{PathQuery, TransactionArg}; +use grovedb_epoch_based_storage_flags::StorageFlags; impl Drive { /// Pushes multiple "delete element" and "insert element operations for items in a given path based on a `PathQuery` to `drive_operations`. diff --git a/packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/v0/mod.rs b/packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/v0/mod.rs index b0caea9c4af..590fce6a40d 100644 --- a/packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/v0/mod.rs +++ b/packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/v0/mod.rs @@ -5,13 +5,13 @@ use crate::fees::op::LowLevelDriveOperation; use crate::fees::op::LowLevelDriveOperation::GroveOperation; use crate::util::grove_operations::pending_grove_operations::pending_grove_operations; use crate::util::grove_operations::{push_drive_operation_result, BatchMoveApplyType}; -use crate::util::storage_flags::StorageFlags; use grovedb::batch::key_info::KeyInfo; use grovedb::batch::{KeyInfoPath, QualifiedGroveDbOp}; use grovedb::operations::delete::DeleteOptions; use grovedb::query_result_type::QueryResultType; use grovedb::BackwardsReferences; use grovedb::{GroveDb, PathQuery, TransactionArg}; +use grovedb_epoch_based_storage_flags::StorageFlags; use grovedb_storage::rocksdb_storage::RocksDbStorage; use platform_version::version::drive_versions::DriveVersion; From 158f12ec15391b3c69c36f2b7410e67574be1598 Mon Sep 17 00:00:00 2001 From: DCG-Claude Date: Tue, 29 Sep 2026 22:53:43 -0500 Subject: [PATCH 13/13] fix(drive): fold the typed cost aggregate under the given grove version GroveDB picks how a basic storage removal joins a sectioned one through a thread-local that it sets only inside its own apply and delete calls. Outside those, the legacy default drops the system section when a basic removal is followed by a sectioned one. combine_cost_operations_with_refund_owners now takes the grove version and scopes its fold to that version's arithmetic, with a test over both operand orders under an ambient legacy selector. Co-Authored-By: Claude Opus 5.5 --- book/src/drive/cost-tracking.md | 2 +- packages/rs-drive/src/fees/op.rs | 113 ++++++++++++++++++++++++++++--- 2 files changed, 104 insertions(+), 11 deletions(-) diff --git a/book/src/drive/cost-tracking.md b/book/src/drive/cost-tracking.md index 50872290464..fbeb7584505 100644 --- a/book/src/drive/cost-tracking.md +++ b/book/src/drive/cost-tracking.md @@ -114,7 +114,7 @@ A fee result that was already computed elsewhere and just needs to be included i ### CalculatedCostOperationWithRefundOwners -A `CalculatedCostOperation` whose sectioned storage removal comes with the typed `RefundOwner` recorded for every carrier key when the removed bytes were split. It is pushed by the batch apply generations that split removed bytes with typed storage flags (`push_drive_operation_result_with_refund_owners`). `operation_cost()` rejects it on purpose: a fee decoder that predates typed owners reaches `operation_cost()` through its catch-all arm and therefore fails closed instead of pricing a refund whose owner it cannot route. `combine_cost_operations` leaves it out for the same reason (folding it into a plain `OperationCost` would erase the owners); `combine_cost_operations_with_refund_owners` sums plain and typed costs together and returns the owner of every sectioned removal key: a plain cost's keys are identities, a typed cost must have recorded an owner for each of its own keys, the system key is never an owner, and one key attributed to two owners is refused. See [Refunds](../fees/overview.md#refunds) for how the owners are recorded. +A `CalculatedCostOperation` whose sectioned storage removal comes with the typed `RefundOwner` recorded for every carrier key when the removed bytes were split. It is pushed by the batch apply generations that split removed bytes with typed storage flags (`push_drive_operation_result_with_refund_owners`). `operation_cost()` rejects it on purpose: a fee decoder that predates typed owners reaches `operation_cost()` through its catch-all arm and therefore fails closed instead of pricing a refund whose owner it cannot route. `combine_cost_operations` leaves it out for the same reason (folding it into a plain `OperationCost` would erase the owners); `combine_cost_operations_with_refund_owners` sums plain and typed costs together and returns the owner of every sectioned removal key: a plain cost's keys are identities, a typed cost must have recorded an owner for each of its own keys, the system key is never an owner, and one key attributed to two owners is refused. It takes the grove version and folds under that version's removal arithmetic, because GroveDB selects how a basic removal joins a sectioned one through a thread-local it sets only inside its own calls. See [Refunds](../fees/overview.md#refunds) for how the owners are recorded. ## BaseOp: Arithmetic Operation Costs diff --git a/packages/rs-drive/src/fees/op.rs b/packages/rs-drive/src/fees/op.rs index ee675f0c31e..c625ffbd08f 100644 --- a/packages/rs-drive/src/fees/op.rs +++ b/packages/rs-drive/src/fees/op.rs @@ -1,4 +1,5 @@ use crate::util::batch::GroveDbOpBatch; +use grovedb_costs::storage_cost::removal::with_basic_sectioned_removal_addition_version; use grovedb_costs::storage_cost::removal::Identifier; use grovedb_costs::storage_cost::removal::StorageRemovedBytes::{ BasicStorageRemoval, NoStorageRemoval, SectionedStorageRemoval, @@ -14,6 +15,7 @@ use grovedb::element::IndexAxis; use grovedb::element::MaxReferenceHop; use grovedb::{batch::QualifiedGroveDbOp, Element, ElementFlags, TreeType}; use grovedb_costs::OperationCost; +use grovedb_version::version::GroveVersion; use crate::error::drive::DriveError; use crate::error::fee::FeeError; @@ -565,8 +567,26 @@ impl LowLevelDriveOperation { /// owner. Owners then merge by carrier key, and one key attributed to two /// different owners, within an input or across inputs, is reported and /// never resolved by picking one. + /// + /// GroveDB selects how a basic removal folds into a sectioned one through + /// a thread-local that it sets only inside its own apply and delete + /// calls, so the fold here is scoped to `grove_version`'s arithmetic + /// rather than to whatever the thread last used. pub fn combine_cost_operations_with_refund_owners( operations: &[LowLevelDriveOperation], + grove_version: &GroveVersion, + ) -> Result<(OperationCost, RefundOwnersByIdentifier), Error> { + with_basic_sectioned_removal_addition_version( + grove_version + .grovedb_versions + .storage_costs + .add_basic_storage_removal_to_sectioned_storage_removal, + || Self::fold_cost_operations_with_refund_owners(operations), + ) + } + + fn fold_cost_operations_with_refund_owners( + operations: &[LowLevelDriveOperation], ) -> Result<(OperationCost, RefundOwnersByIdentifier), Error> { let mut cost = OperationCost::default(); let mut owners = RefundOwnersByIdentifier::new(); @@ -2088,12 +2108,14 @@ mod tests { use super::*; use dpp::identifier::Identifier; use grovedb_costs::storage_cost::removal::{ - StorageRemovalPerEpochByIdentifier, StorageRemovedBytes, + StorageRemovalPerEpochByIdentifier, StorageRemovedBytes, UNKNOWN_EPOCH, }; use grovedb_costs::storage_cost::StorageCost; + use grovedb_version::version::v1::GROVE_V1; use intmap::IntMap; use platform_version::version::fee::storage::FeeStorageVersion; use platform_version::version::fee::FeeVersion; + use platform_version::version::PlatformVersion; /// Helper to get the canonical fee version used across these tests. fn fee_version() -> &'static FeeVersion { @@ -2547,6 +2569,10 @@ mod tests { assert_eq!(combined, cost, "only the plain cost is folded"); } + fn grove_version() -> &'static GroveVersion { + &PlatformVersion::latest().drive.grove_version + } + fn sectioned_cost(entries: &[([u8; 32], u32)]) -> OperationCost { let mut removal = StorageRemovalPerEpochByIdentifier::new(); for (key, bytes) in entries { @@ -2591,8 +2617,11 @@ mod tests { ]; let (combined, owners) = - LowLevelDriveOperation::combine_cost_operations_with_refund_owners(&operations) - .expect("should combine"); + LowLevelDriveOperation::combine_cost_operations_with_refund_owners( + &operations, + grove_version(), + ) + .expect("should combine"); assert_eq!(combined.seek_count, 3); let SectionedStorageRemoval(removal) = &combined.storage_cost.removed_bytes else { @@ -2641,9 +2670,11 @@ mod tests { )]), }]; - let (_, owners) = - LowLevelDriveOperation::combine_cost_operations_with_refund_owners(&operations) - .expect("should combine"); + let (_, owners) = LowLevelDriveOperation::combine_cost_operations_with_refund_owners( + &operations, + grove_version(), + ) + .expect("should combine"); assert!(owners.is_empty()); } @@ -2667,8 +2698,10 @@ mod tests { }, ]; - let result = - LowLevelDriveOperation::combine_cost_operations_with_refund_owners(&operations); + let result = LowLevelDriveOperation::combine_cost_operations_with_refund_owners( + &operations, + grove_version(), + ); assert!(matches!( result, @@ -2693,7 +2726,10 @@ mod tests { }, ]; assert!(matches!( - LowLevelDriveOperation::combine_cost_operations_with_refund_owners(&plain_then_typed), + LowLevelDriveOperation::combine_cost_operations_with_refund_owners( + &plain_then_typed, + grove_version() + ), Err(Error::Drive(DriveError::CorruptedCodeExecution(_))) )); @@ -2712,11 +2748,68 @@ mod tests { }, ]; assert!(matches!( - LowLevelDriveOperation::combine_cost_operations_with_refund_owners(&typed_conflict), + LowLevelDriveOperation::combine_cost_operations_with_refund_owners( + &typed_conflict, + grove_version() + ), Err(Error::Drive(DriveError::CorruptedCodeExecution(_))) )); } + /// GroveDB sets its removal arithmetic only inside its own calls, so a + /// fold outside one would otherwise run under whatever the thread holds + /// (the legacy default drops the system section when a basic removal + /// meets a sectioned one). The aggregate follows the grove version it is + /// given, in both operand orders, whatever the ambient selector is. + #[test] + fn should_fold_basic_and_sectioned_removals_under_the_given_grove_version() { + let bucket = RefundOwner::ContractBucket { + contract_id: Identifier::from([1u8; 32]), + position: 0, + }; + let basic = || { + CalculatedCostOperation(OperationCost { + storage_cost: StorageCost { + removed_bytes: StorageRemovedBytes::BasicStorageRemoval(10), + ..Default::default() + }, + ..Default::default() + }) + }; + let typed = || CalculatedCostOperationWithRefundOwners { + cost: sectioned_cost(&[(SYSTEM_REFUND_CARRIER_KEY, 5), (bucket.removal_key(), 40)]), + refund_owners: BTreeMap::from([(bucket.removal_key(), bucket)]), + }; + let removal_of = |operations: &[LowLevelDriveOperation], grove_version: &GroveVersion| { + // an ambient legacy selector must not leak into the fold + with_basic_sectioned_removal_addition_version(0, || { + LowLevelDriveOperation::combine_cost_operations_with_refund_owners( + operations, + grove_version, + ) + }) + .expect("should combine") + .0 + .storage_cost + .removed_bytes + }; + let expected_system = IntMap::from_iter([(0u16, 5u32), (UNKNOWN_EPOCH, 10)]); + + for operations in [vec![basic(), typed()], vec![typed(), basic()]] { + let SectionedStorageRemoval(removal) = removal_of(&operations, grove_version()) else { + panic!("sectioned removals stay sectioned"); + }; + assert_eq!(removal[&SYSTEM_REFUND_CARRIER_KEY], expected_system); + assert_eq!(removal[&bucket.removal_key()].get(0u16), Some(&40)); + } + + // the historical arithmetic is still what an older grove version gets + let SectionedStorageRemoval(legacy) = removal_of(&[basic(), typed()], &GROVE_V1) else { + panic!("sectioned removals stay sectioned"); + }; + assert!(!legacy.contains_key(&SYSTEM_REFUND_CARRIER_KEY)); + } + // --------------------------------------------------------------- // 6. combine_cost_operations — filter and sum // ---------------------------------------------------------------