Skip to content

Conversation

@QuantumExplorer
Copy link
Member

@QuantumExplorer QuantumExplorer commented Dec 29, 2025

What was done?

New Query API

  • Added getRecentAddressBalanceChanges gRPC endpoint that returns address balance changes from a specified block height onwards
  • Returns both SetBalance (absolute balance after spending) and AddToBalance (credits added) operations per address per block

Address Balance Change Tracking

  • Added CreditOperation enum in DPP (SetCredits, AddToCredits) to represent how address balances are modified
  • Extended StateTransitionExecutionResult::SuccessfulExecution to include address_balance_changes map
  • Extended StateTransitionsProcessingResult to aggregate address balance changes across all state transitions in a block
  • Added add_address_balances_in_update() method with proper merging logic for combining operations (Set+Set, Set+Add, Add+Add, Add+Set)

Historical Storage

  • Added new SavedBlockTransactions root tree in GroveDB for storing historical data
  • Added AddressBalances subtree under SavedBlockTransactions to store per-block address balance changes
  • Implemented store_address_balances_for_block to serialize and store balance changes keyed by block height (big-endian for proper ordering)
  • Implemented fetch_recent_address_balance_changes and prove_recent_address_balance_changes for retrieval

Execution Event Updates

  • Added added_to_balance_outputs field to ExecutionEvent::Paid to track outputs from IdentityCreditTransferToAddresses
  • Updated execute_event to populate address_balances map during execution

Test Updates

  • Added comprehensive tests in address_tests.rs covering:
    • run_chain_address_withdrawal_transitions - withdrawal operations
    • run_chain_identity_credit_transfer_to_addresses - identity to address transfers
    • run_chain_all_address_transitions - comprehensive test with all 5 address state transition types
  • Updated existing tests to use new struct-style execution result variants

How Has This Been Tested?

  • All 9 address tests pass:
    • run_chain_address_transitions
    • run_chain_identity_to_addresses_transitions
    • run_chain_identity_create_from_addresses_transitions
    • run_chain_address_transitions_with_checkpoints
    • run_chain_address_transitions_with_checkpoints_stop_and_restart
    • run_chain_address_withdrawal_transitions
    • run_chain_identity_credit_transfer_to_addresses
    • run_chain_all_address_transitions
    • run_chain_address_transitions_with_proof_signature_verification

Tests verify:

  • Address balance changes are correctly recorded for all address-related state transitions
  • The query API returns correct SetBalance and AddToBalance operations
  • Balance changes from funding, transfers, withdrawals, and identity operations are all tracked

Breaking Changes

Yes, a new tree was introduced.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Summary by CodeRabbit

  • New Features
    • Added getRecentAddressBalanceChanges RPC to fetch recent per-address balance changes from a given block height, with optional proof support.
    • Track and persist per-block address balance updates (set/add) so clients can query recent balance-change history.
  • Chores
    • Versioning and feature flags updated to expose saved-block-transactions and recent-address-balance-change capabilities.

✏️ Tip: You can customize this high-level summary in your review settings.

@github-actions github-actions bot added this to the v3.0.0 milestone Dec 29, 2025
@github-actions
Copy link

github-actions bot commented Dec 29, 2025

❌ gRPC Query Coverage Report

================================================================================
gRPC Query Coverage Report - NEW QUERIES ONLY
================================================================================

Total queries in proto: 52
Previously known queries: 47
New queries found: 5

================================================================================

New Query Implementation Status:
--------------------------------------------------------------------------------
✓ getAddressInfo                                /home/runner/work/platform/platform/packages/rs-sdk/src/platform/query.rs
✓ getAddressesBranchState                       /home/runner/work/platform/platform/packages/rs-sdk/src/platform/address_sync/mod.rs
✓ getAddressesInfos                             /home/runner/work/platform/platform/packages/rs-sdk/src/platform/fetch_many.rs
✓ getAddressesTrunkState                        /home/runner/work/platform/platform/packages/rs-sdk/src/platform/address_sync/mod.rs
✗ getRecentAddressBalanceChanges                NOT FOUND

================================================================================
Summary:
--------------------------------------------------------------------------------
New queries implemented: 4 (80.0%)
New queries missing: 1 (20.0%)

Total known queries: 52
  - Implemented: 48
  - Not implemented: 3
  - Excluded: 1

Not implemented queries:
  - getConsensusParams
  - getRecentAddressBalanceChanges
  - getTokenPreProgrammedDistributions

Missing NEW queries:
  - getRecentAddressBalanceChanges

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 29, 2025

📝 Walkthrough

Walkthrough

Adds end-to-end support for tracking, storing, and querying recent per-address balance changes: protocol/gRPC messages, execution-time balance-change accumulation, storage in a new SavedBlockTransactions tree, Drive read/write methods, version gating, and test updates.

Changes

Cohort / File(s) Summary
Protocol / gRPC
packages/dapi-grpc/protos/platform/v0/platform.proto, packages/dapi-grpc/build.rs
New RPC getRecentAddressBalanceChanges and protobuf messages; register new versioned request/response entries.
Platform service & query wiring
packages/rs-dapi/src/services/platform_service/mod.rs, packages/rs-drive-abci/src/query/service.rs, packages/rs-drive-abci/src/query/address_funds/recent_address_balance_changes/{mod.rs,v0/mod.rs}
Added PlatformService handler and versioned query implementation for recent address balance changes (prove/no-prove modes, V0 dispatch and validation).
Drive: SavedBlockTransactions storage
packages/rs-drive/src/drive/saved_block_transactions/*, packages/rs-drive/src/drive/mod.rs, packages/rs-drive/src/drive/saved_block_transactions/queries.rs
New SavedBlockTransactions module, root-tree variant (36), path helpers, and versioned fetch/prove/store APIs for per-block address balance maps.
Drive — fetch/prove implementations
packages/rs-drive/src/drive/saved_block_transactions/fetch_address_balances/{mod.rs,v0/mod.rs}, packages/rs-drive/src/drive/saved_block_transactions/store_address_balances/{mod.rs,v0/mod.rs}
Implement v0: serialize/deserialize BTreeMap<PlatformAddress, CreditOperation>, perform path queries and proved queries, and store serialized address-balance data keyed by block height.
Execution: track & persist address balance changes
packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/{mod.rs,store_address_balances_to_recent_block_storage/*,execute_event/*,process_raw_state_transitions/v0/mod.rs}
Thread optional address_balances_in_update map through execute_event; collect per-address CreditOperation updates during event execution; add versioned store_address_balances_to_recent_block_storage and call it after block processing.
Types: Credit operations & events
packages/rs-dpp/src/balances/credits.rs, packages/rs-drive-abci/src/execution/types/execution_event/mod.rs, packages/rs-drive-abci/src/execution/types/block_execution_context/v0/mod.rs
New CreditOperation enum (Set/Add); ExecutionEvent::Paid now carries optional added_to_balance_outputs; BlockExecutionContextV0 adds block_address_balance_changes field.
StateTransitionExecutionResult & aggregation
packages/rs-drive-abci/src/platform_types/state_transitions_processing_result/mod.rs, packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/process_raw_state_transitions/v0/mod.rs
Changed SuccessfulExecution and PaidConsensusError from tuple variants to struct-like variants; SuccessfulExecution now includes address_balance_changes; StateTransitionsProcessingResult gains address_balances_updated and merging helper.
Drive initialization & paths
packages/rs-drive/src/drive/initialization/{v0/mod.rs,v2/mod.rs}, packages/rs-drive/src/util/batch/grovedb_op_batch/mod.rs
Initialize SavedBlockTransactions subtree in startup code; add SavedBlockTransactionsRoot internal path variant and adjust related proofs/expectations.
Platform/versioning
packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_method_versions/*, packages/rs-platform-version/src/version/drive_abci_versions/drive_abci_query_versions/*, packages/rs-platform-version/src/version/drive_versions/*, packages/rs-platform-version/src/version/mocks/*
Add version fields and feature bounds for store_address_balances_to_recent_block_storage and recent_address_balance_changes; introduce DriveSavedBlockTransactionsMethodVersions and wire into drive version constants.
Query surface & tests
packages/rs-drive-abci/tests/strategy_tests/*.rs, packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/**/*, packages/rs-drive-abci/src/test/helpers/fast_forward_to_block.rs, packages/rs-drive/tests/strategy_tests/strategy.rs
Extensive test updates: pattern matching adjusted from tuple to struct variants across many tests; new/expanded address-balance-focused strategy tests validate per-block balance-change queries and proofs.
Misc: small refactors & pattern updates
packages/rs-drive-abci/src/abci/*, packages/rs-drive-abci/src/execution/check_tx/v0/mod.rs, packages/rs-drive-abci/src/execution/engine/run_block_proposal/v0/mod.rs, etc.
Syntactic refactors switching tuple-patterns to struct-patterns, added None args where appropriate, and integrated new block_address_balance_changes initialization.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant gRPC_Service as gRPC Service
    participant Platform_Query as Platform Query
    participant Drive as Drive Storage
    participant GroveDB as Grove DB

    Client->>gRPC_Service: getRecentAddressBalanceChanges(start_height, prove)
    activate gRPC_Service
    gRPC_Service->>Platform_Query: query_recent_address_balance_changes(request)
    activate Platform_Query
    Platform_Query->>Platform_Query: validate request version & bounds
    alt prove == true
        Platform_Query->>Drive: prove_recent_address_balance_changes(start_height, limit)
        activate Drive
        Drive->>GroveDB: grove_get_proved_path_query(SavedBlockTransactions/address_balances)
        GroveDB-->>Drive: proof bytes
        Drive-->>Platform_Query: proof + metadata
        deactivate Drive
    else
        Platform_Query->>Drive: fetch_recent_address_balance_changes(start_height, limit)
        activate Drive
        Drive->>GroveDB: grove_get_raw_path_query(SavedBlockTransactions/address_balances)
        GroveDB-->>Drive: [(height, serialized_map)]*
        Drive->>Drive: deserialize to AddressBalanceChange structures
        Drive-->>Platform_Query: AddressBalanceUpdateEntries + metadata
        deactivate Drive
    end
    Platform_Query-->>gRPC_Service: GetRecentAddressBalanceChangesResponse
    deactivate Platform_Query
    gRPC_Service-->>Client: gRPC Response
    deactivate gRPC_Service
Loading
sequenceDiagram
    participant BlockProcessor
    participant StateTransitions as State Transition Processor
    participant Executor as Execute Event
    participant BalanceMap as Balance Tracker
    participant DriveStore as SavedBlockTransactions Store
    participant GroveDB as Grove DB

    BlockProcessor->>StateTransitions: process_raw_state_transitions()
    activate StateTransitions
    StateTransitions->>StateTransitions: address_balances = BTreeMap::new()
    loop for each state transition
        StateTransitions->>Executor: execute_event(Some(&mut address_balances))
        activate Executor
        alt event is Paid / transfers
            Executor->>BalanceMap: record SetCredits/AddToCredits per address
            note right of BalanceMap: consolidation rules applied<br/>Set+Set → last Set<br/>Set+Add → Set with combined value<br/>Add+Add → saturating add<br/>Add+Set → Set wins
            BalanceMap-->>Executor: updated map
        end
        Executor-->>StateTransitions: returned with address_balance_changes
        deactivate Executor
    end
    StateTransitions->>BlockProcessor: StateTransitionsProcessingResult (includes address_balances_updated)
    deactivate StateTransitions
    BlockProcessor->>DriveStore: store_address_balances_to_recent_block_storage(address_balances, block_info)
    activate DriveStore
    DriveStore->>GroveDB: grove_insert(SavedBlockTransactions/address_balances, key=height, value=serialized_map)
    GroveDB-->>DriveStore: ok
    deactivate DriveStore
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐇 I hopped through blocks with ledger in paw,

Collected each change that I happened to saw.
Set and add, then saved by height,
Now queries fetch balances — oh what a sight! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(platform)!: historical address balance storage' directly and specifically describes the main feature added—a new historical storage system for address balance changes, which is the primary focus of this large changeset.
✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/rs-drive-abci/src/query/service.rs (1)

120-135: counter in the height‑sync loop is re‑initialized each iteration (saturation guard never triggers)

In handle_blocking_query, the inner loop that waits for platform_state.last_committed_block_height() to match committed_block_height_guard declares let mut counter = 0; inside the loop body. That re‑initializes counter on every iteration, so if counter >= 100 is effectively unreachable and the intended “give up after ~1s and restart” behavior never happens. In a pathological mismatch, this can spin indefinitely.

You can fix this by moving counter outside the inner loop so it actually counts iterations:

Proposed fix for the wait‑loop
-                let mut needs_restart = false;
-
-                loop {
-                    let committed_block_height_guard = platform
-                        .committed_block_height_guard
-                        .load(Ordering::Relaxed);
-                    let mut counter = 0;
+                let mut needs_restart = false;
+                let mut counter = 0;
+
+                loop {
+                    let committed_block_height_guard = platform
+                        .committed_block_height_guard
+                        .load(Ordering::Relaxed);
@@
-                    } else {
-                        counter += 1;
-                        sleep(Duration::from_millis(10))
-                    }
-
-                    // We try for up to 1 second
-                    if counter >= 100 {
-                        query_counter += 1;
-                        needs_restart = true;
-                        break;
-                    }
+                    } else {
+                        counter += 1;
+                        sleep(Duration::from_millis(10));
+                        // We try for up to 1 second
+                        if counter >= 100 {
+                            query_counter += 1;
+                            needs_restart = true;
+                            break;
+                        }
+                    }
                 }
packages/rs-drive/src/drive/mod.rs (1)

274-296: Missing AddressBalances and GroupActions cases in TryFrom<u8>.

The TryFrom<u8> implementation is missing cases for:

  • 56 => Ok(RootTree::AddressBalances)
  • 88 => Ok(RootTree::GroupActions)

These are present in the enum definition and the From<RootTree> for &'static [u8; 1] implementation but missing here.

🔎 Proposed fix
             40 => Ok(RootTree::PreFundedSpecializedBalances),
             36 => Ok(RootTree::SavedBlockTransactions),
+            56 => Ok(RootTree::AddressBalances),
             72 => Ok(RootTree::SpentAssetLockTransactions),
             104 => Ok(RootTree::Misc),
             80 => Ok(RootTree::WithdrawalTransactions),
             96 => Ok(RootTree::Balances),
             16 => Ok(RootTree::Tokens),
             120 => Ok(RootTree::Versions),
             112 => Ok(RootTree::Votes),
+            88 => Ok(RootTree::GroupActions),
             _ => Err(Error::Drive(DriveError::NotSupported(
🧹 Nitpick comments (14)
packages/rs-drive-abci/tests/strategy_tests/test_cases/address_tests.rs (4)

122-123: Test naming convention: Consider using should_* prefix.

Per coding guidelines, test names should start with "should …" (e.g., should_execute_address_transitions). However, the existing run_chain_* pattern may be established in this test suite. Consider aligning with the guideline for new tests or documenting the team's preferred convention.


526-526: Consider removing unused variable.

The _platform_version variable is defined but never used in this test. Consider removing it to reduce noise.

🔎 Proposed fix
-    let _platform_version = PlatformVersion::latest();

1020-1020: Consider moving import to module level.

The use dpp::dashcore::Txid; import inside the function is valid Rust but unconventional. Consider moving it to the module-level imports for consistency with the rest of the file.


1619-1648: Minor: Comment numbering inconsistency.

The operation comments say "5. Create identity" and "6. Top up identity" but should be "4." and "5." since operation 4 (IdentityCreditTransferToAddresses) was skipped. This is a minor documentation inconsistency.

🔎 Proposed fix
                     // Note: IdentityTransferToAddresses requires identities with transfer keys
                     // which need special setup - skipped for now
-                    // 5. Create identity from address funds
+                    // 4. Create identity from address funds
                     Operation {
                         op_type: OperationType::IdentityCreateFromAddresses(
                             dash_to_credits!(5)..=dash_to_credits!(10),
                             Some(dash_to_credits!(1)..=dash_to_credits!(2)),
                             None,
                             3,
                             [(
                                 Purpose::AUTHENTICATION,
                                 [(SecurityLevel::CRITICAL, vec![KeyType::ECDSA_SECP256K1])].into(),
                             )]
                             .into(),
                         ),
                         frequency: Frequency {
                             times_per_block_range: 0..2,
                             chance_per_block: Some(0.3),
                         },
                     },
-                    // 6. Top up identity from address funds
+                    // 5. Top up identity from address funds
                     Operation {
packages/rs-drive/src/drive/saved_block_transactions/fetch_address_balances/v0/mod.rs (1)

80-102: Consider extracting shared query construction logic.

The query construction (lines 88-94) duplicates the logic in fetch_recent_address_balance_changes_v0 (lines 26-32). Consider extracting a private helper method to build the PathQuery from start_height and limit.

🔎 Suggested helper extraction
fn build_address_balance_changes_path_query(start_height: u64, limit: Option<u16>) -> PathQuery {
    let path = Self::saved_block_transactions_address_balances_path_vec();
    let mut query = Query::new();
    query.insert_range_from(start_height.to_be_bytes().to_vec()..);
    PathQuery::new(path, SizedQuery::new(query, limit, None))
}
packages/rs-drive-abci/src/platform_types/state_transitions_processing_result/mod.rs (1)

117-128: Consider taking ownership of execution_result to avoid clone.

The address_balance_changes.clone() on line 127 is necessary because add() takes execution_result by reference. If add() took ownership instead, the clone could be avoided for the successful execution case. This is a minor performance consideration.

packages/rs-drive/src/drive/initialization/v0/mod.rs (1)

319-319: Updated root count and proof sizes for latest protocol version are consistent

  • Line 319: Expecting elements.len() == 16 for PlatformVersion::latest() aligns with the additional root trees introduced in newer protocols (e.g. AddressBalances, SavedBlockTransactions on top of earlier roots).
  • Line 1141: The adjusted proof length for PreFundedSpecializedBalances in the latest version reflects the deeper/more complex root layout; using the exact byte length keeps this test a strict regression guard on the merk structure.
  • Lines 1250–1269: The new Merk Level 4 check for RootTree::SavedBlockTransactions with proof length 286 correctly exercises the added subtree and ensures the new root participates in proofs as expected.

Given these tests are deliberately layout‑sensitive, the hard-coded lengths look intentional and appropriate for guarding structural regressions.

If proof encoding changes frequently in future, consider factoring these “expected proof len” constants behind small helpers or comments tied to a diagram of the merk tree, to make future updates less error‑prone, but this is optional.

Also applies to: 1141-1141, 1250-1269

packages/rs-dpp/src/balances/credits.rs (1)

42-49: LGTM! Well-designed CreditOperation enum.

The CreditOperation enum is simple, clear, and appropriately derives all necessary traits including Copy (since Credits is u64) and bincode serialization traits for storage.

Optional: Consider adding helper methods

You might consider adding convenience methods to apply operations or combine them:

+impl CreditOperation {
+    /// Apply this operation to a base credit amount
+    pub fn apply(&self, base: Credits) -> Option<Credits> {
+        match self {
+            CreditOperation::SetCredits(amount) => Some(*amount),
+            CreditOperation::AddToCredits(amount) => base.checked_add(*amount),
+        }
+    }
+}

This would centralize the logic for applying operations, though it may not be necessary if operations are always applied at higher layers.

packages/rs-drive-abci/src/execution/engine/run_block_proposal/v0/mod.rs (1)

370-370: Consider using Default::default() for consistency.

Line 370 uses std::collections::BTreeMap::new() while other similar initializations in the codebase (e.g., line 276 in add_process_epoch_change_operations/v0/mod.rs) use Default::default().

Suggestion for consistency
-            block_address_balance_changes: std::collections::BTreeMap::new(),
+            block_address_balance_changes: Default::default(),

This aligns with the pattern used in test helpers and avoids the fully-qualified type path.

packages/rs-drive/src/drive/saved_block_transactions/mod.rs (1)

6-6: Consider explicit re-exports instead of glob import.

Using pub use queries::* re-exports all public items from the queries module, which could unintentionally expand the public API surface. If only specific items are intended for external use, consider listing them explicitly for better API control and documentation.

🔎 Example of explicit re-exports
-pub use queries::*;
+pub use queries::{specific_query_fn, another_query_fn}; // List intended exports
packages/rs-drive-abci/src/execution/types/block_execution_context/v0/mod.rs (1)

2-2: Add accessors for block_address_balance_changes to keep the traits consistent

You added block_address_balance_changes as a public field, but the BlockExecutionContextV0Getters / BlockExecutionContextV0MutableGetters / BlockExecutionContextV0OwnedGetters and BlockExecutionContextV0Setters traits don’t expose it. For API consistency and future refactors, it’s worth adding corresponding methods so callers don’t need to reach into the struct field directly.

Example trait & impl additions
 pub trait BlockExecutionContextV0Getters {
@@
     /// Returns a reference of the withdrawal_transactions field.
     fn unsigned_withdrawal_transactions(&self) -> &UnsignedWithdrawalTxs;
+
+    /// Returns a reference of the recent address balance changes.
+    fn block_address_balance_changes(
+        &self,
+    ) -> &BTreeMap<PlatformAddress, Credits>;
@@
 impl BlockExecutionContextV0Getters for BlockExecutionContextV0 {
@@
     fn unsigned_withdrawal_transactions(&self) -> &UnsignedWithdrawalTxs {
         &self.unsigned_withdrawal_transactions
     }
+
+    fn block_address_balance_changes(
+        &self,
+    ) -> &BTreeMap<PlatformAddress, Credits> {
+        &self.block_address_balance_changes
+    }
 }

Similar small additions can be made for the mutable, owned, and setter traits if you want full symmetry.

Also applies to: 8-9, 21-22

packages/rs-drive-abci/src/execution/types/execution_event/mod.rs (1)

416-444: Confirm removed_balance derivation for IdentityCreditTransferToAddressesAction

Here removed_balance is computed as recipient_addresses().values().sum(), and the same map is stored in added_to_balance_outputs. This is reasonable, but it assumes the sum of recipient amounts is the canonical amount to remove from the identity.

If the action type already tracks a total transfer amount or enforces invariants between the declared amount and the per‑address map, it may be safer either to:

  • derive removed_balance from that canonical field, or
  • add an internal debug assertion that the declared total equals recipient_addresses().values().sum().

That would prevent silent drift if the action structure changes in future.

packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/execute_event/v0/mod.rs (1)

101-227: Address balance tracking in paid_from_address_inputs_and_outputs is logically sound

The new address_balances_in_update handling for inputs/outputs correctly captures net per-address changes with coherent merge rules:

  • Outputs: you record AddToCredits(adjusted_amount) (post-fee), and combine:
    • SetCredits + AddToCreditsSetCredits(set_val + adjusted_amount)
    • AddToCredits + AddToCreditsAddToCredits(sum) (saturating).
  • Inputs: you always emit SetCredits(adjusted_remaining), overwriting any prior Add or Set, so input addresses end up with an absolute final balance, which is appropriate when you know the remaining balance after fees.
  • Processing outputs before inputs ensures that when an address appears in both, the final operation is a SetCredits from the input pass, as intended.

Behavioral semantics (tracking net changes, not intermediate fee deductions) look consistent with the CreditOperation design. The duplicated merge logic for outputs and inputs could be factored into a small helper for clarity, but that's optional.

Also applies to: 229-267

packages/rs-drive-abci/src/execution/platform_events/state_transition_processing/process_raw_state_transitions/v0/mod.rs (1)

223-293: Address balance tracking and struct-like execution results are wired correctly

  • Invalid transitions:
    • You pass None for address_balances_in_update into execute_event, and map paid/ free/unpaid cases to PaidConsensusError { error, actual_fees } or UnpaidConsensusError exactly as before, which is appropriate since you don’t want to expose address deltas for invalid STs.
  • Valid transitions:
    • You initialize let mut address_balances = BTreeMap::new(); and pass Some(&mut address_balances) into execute_event, so downstream code can accumulate CreditOperations per address.
    • SuccessfulPaidExecution(estimated_fees, actual_fees) maps to:
      • SuccessfulExecution { estimated_fees, fee_result: actual_fees, address_balance_changes: address_balances }, which pairs the actual fee result with the collected address changes.
    • UnsuccessfulPaidExecution correctly yields PaidConsensusError { error, actual_fees } without address changes, aligning with the idea that these STs are invalid from a consensus perspective.
    • SuccessfulFreeExecution returns SuccessfulExecution { estimated_fees: None, fee_result: FeeResult::default(), address_balance_changes: BTreeMap::new() }, which keeps free transitions free and with no tracked address changes.

One minor improvement: in the SuccessfulFreeExecution arm you could reuse the local address_balances instead of constructing a new BTreeMap::new(). Today both are empty, but using address_balances would automatically propagate any future address tracking for free events without touching this code.

Also applies to: 362-482

Comment on lines +1977 to +1983
message AddressBalanceChange {
bytes address = 1;
oneof operation {
uint64 set_balance = 2;
uint64 add_to_balance = 3;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Add jstype = JS_STRING to uint64 fields for JavaScript compatibility.

The set_balance and add_to_balance fields are uint64 but lack the jstype = JS_STRING annotation. JavaScript's Number type loses precision for integers beyond 2^53. Other balance fields in this file consistently use this annotation (e.g., GetIdentityBalanceResponseV0.balance, GetPrefundedSpecializedBalanceResponseV0.balance).

Proposed fix
 message AddressBalanceChange {
   bytes address = 1;
   oneof operation {
-    uint64 set_balance = 2;
-    uint64 add_to_balance = 3;
+    uint64 set_balance = 2 [jstype = JS_STRING];
+    uint64 add_to_balance = 3 [jstype = JS_STRING];
   }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
message AddressBalanceChange {
bytes address = 1;
oneof operation {
uint64 set_balance = 2;
uint64 add_to_balance = 3;
}
}
message AddressBalanceChange {
bytes address = 1;
oneof operation {
uint64 set_balance = 2 [jstype = JS_STRING];
uint64 add_to_balance = 3 [jstype = JS_STRING];
}
}
🤖 Prompt for AI Agents
In packages/dapi-grpc/protos/platform/v0/platform.proto around lines 1977 to
1983, the uint64 fields set_balance and add_to_balance in AddressBalanceChange
must include the protobuf field option jstype = JS_STRING to prevent precision
loss in JavaScript; update the field declarations to add [jstype = JS_STRING]
for both uint64 fields and then regenerate the JavaScript/TS gRPC bindings
(re-run protoc / your build step) so the change takes effect.

Comment on lines +1985 to +1988
message BlockAddressBalanceChanges {
uint64 block_height = 1;
repeated AddressBalanceChange changes = 2;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Add jstype = JS_STRING to block_height for consistency.

Other block_height fields in this file (e.g., EpochInfo.first_block_height, FinalizedEpochInfo.first_block_height) use jstype = JS_STRING.

Proposed fix
 message BlockAddressBalanceChanges {
-  uint64 block_height = 1;
+  uint64 block_height = 1 [jstype = JS_STRING];
   repeated AddressBalanceChange changes = 2;
 }
🤖 Prompt for AI Agents
packages/dapi-grpc/protos/platform/v0/platform.proto around lines 1985-1988: the
uint64 block_height field in BlockAddressBalanceChanges must include the
protobuf JS type option for consistency with other block_height fields; update
the field declaration to add the jstype option (e.g., change the field to
include [jstype = JS_STRING]) so it mirrors the other occurrences in this file;
if the file does not already import or enable the jstype option, ensure the same
import/option usage present elsewhere in this proto is applied here.

Comment on lines +2054 to +2060
message GetRecentAddressBalanceChangesRequest {
message GetRecentAddressBalanceChangesRequestV0 {
uint64 start_height = 1;
bool prove = 2;
}
oneof version { GetRecentAddressBalanceChangesRequestV0 v0 = 1; }
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the proto file to see the current implementation and context
head -2100 packages/dapi-grpc/protos/platform/v0/platform.proto | tail -150

Repository: dashpay/platform

Length of output: 3655


🏁 Script executed:

# Search for similar request messages with pagination patterns
rg "GetEpochsInfoRequestV0|GetFinalizedEpochInfosRequestV0" packages/dapi-grpc/protos/platform/v0/platform.proto -A 5

Repository: dashpay/platform

Length of output: 1197


🏁 Script executed:

# Check jstype usage patterns for uint64 fields in this proto file
rg "uint64.*jstype|jstype.*uint64" packages/dapi-grpc/protos/platform/v0/platform.proto -B 1 -A 1

Repository: dashpay/platform

Length of output: 3381


🏁 Script executed:

# Search for all occurrences of jstype in the file to understand the pattern
rg "jstype" packages/dapi-grpc/protos/platform/v0/platform.proto | head -20

Repository: dashpay/platform

Length of output: 1340


🏁 Script executed:

# Look for any backend code that might enforce limits on this request
fd . packages/dapi-rpc --type f | head -5
rg "GetRecentAddressBalanceChanges" packages/ -l --type js --type ts --type proto

Repository: dashpay/platform

Length of output: 194


🏁 Script executed:

# Search for backend implementation of GetRecentAddressBalanceChanges
rg "GetRecentAddressBalanceChanges" packages/ --type ts --type js -A 10 -B 2 | head -100

Repository: dashpay/platform

Length of output: 42


🏁 Script executed:

# Look for any size limits or response constraints in backend code
rg "start_height|RecentAddressBalanceChanges" packages/ -A 3 -B 3 --type ts --type js | head -150

Repository: dashpay/platform

Length of output: 42


🏁 Script executed:

# Check if there's a standard max limit pattern in RPC handlers
rg "limit|MAX_" packages/dapi-core packages/dapi-client --type ts --type js | grep -i "block\|height\|balance" | head -20

Repository: dashpay/platform

Length of output: 179


🏁 Script executed:

# Search for any comments or documentation about response size limits
rg "unbounded|response.*limit|max.*block" packages/ --type ts --type proto -i | head -20

Repository: dashpay/platform

Length of output: 89


🏁 Script executed:

# Find where GetRecentAddressBalanceChanges is used in the codebase
rg "GetRecentAddressBalanceChanges" packages/ -l

Repository: dashpay/platform

Length of output: 483


🏁 Script executed:

# List main packages to understand structure
ls -la packages/ | head -30

Repository: dashpay/platform

Length of output: 1937


🏁 Script executed:

# Search for any gRPC service definitions that might handle this
rg "service.*Platform|GetRecentAddressBalanceChanges" packages/dapi-grpc -A 5 -B 2

Repository: dashpay/platform

Length of output: 50373


🏁 Script executed:

# Check if there's validation or size limit logic in proto files
rg "maxlen|max_size|validate.rules|deprecated" packages/dapi-grpc/protos/platform/v0/platform.proto | head -20

Repository: dashpay/platform

Length of output: 42


🏁 Script executed:

# Look at the response structure to see if there's implicit limiting
rg "AddressBalanceUpdateEntries|BlockAddressBalanceChanges" packages/dapi-grpc/protos/platform/v0/platform.proto -B 3 -A 10

Repository: dashpay/platform

Length of output: 815


🏁 Script executed:

# Check the Rust backend implementation
wc -l packages/rs-drive-abci/src/query/address_funds/recent_address_balance_changes/v0/mod.rs

Repository: dashpay/platform

Length of output: 150


🏁 Script executed:

# Read the Rust backend implementation
cat packages/rs-drive-abci/src/query/address_funds/recent_address_balance_changes/v0/mod.rs

Repository: dashpay/platform

Length of output: 4039


🏁 Script executed:

# Also check the parent mod.rs
cat packages/rs-drive-abci/src/query/address_funds/recent_address_balance_changes/mod.rs

Repository: dashpay/platform

Length of output: 2622


🏁 Script executed:

# Search for any size limits or max block constraints in this query handler
rg "limit|max|threshold" packages/rs-drive-abci/src/query/address_funds/ -B 2 -A 2

Repository: dashpay/platform

Length of output: 5161


Add jstype = JS_STRING to the start_height field.

The start_height field should include the jstype = JS_STRING annotation for JavaScript compatibility, consistent with other uint64 block height fields throughout the proto file (e.g., checkpoint_height in GetAddressesBranchStateRequest, height in ResponseMetadata).

The pagination concern in the original comment is not applicable—the backend enforces a hardcoded limit of 100 blocks per request, preventing unbounded responses regardless of the start_height parameter.

 message GetRecentAddressBalanceChangesRequest {
   message GetRecentAddressBalanceChangesRequestV0 {
-    uint64 start_height = 1;
+    uint64 start_height = 1 [jstype = JS_STRING];
     bool prove = 2;
   }
   oneof version { GetRecentAddressBalanceChangesRequestV0 v0 = 1; }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
message GetRecentAddressBalanceChangesRequest {
message GetRecentAddressBalanceChangesRequestV0 {
uint64 start_height = 1;
bool prove = 2;
}
oneof version { GetRecentAddressBalanceChangesRequestV0 v0 = 1; }
}
message GetRecentAddressBalanceChangesRequest {
message GetRecentAddressBalanceChangesRequestV0 {
uint64 start_height = 1 [jstype = JS_STRING];
bool prove = 2;
}
oneof version { GetRecentAddressBalanceChangesRequestV0 v0 = 1; }
}
🤖 Prompt for AI Agents
In packages/dapi-grpc/protos/platform/v0/platform.proto around lines 2054 to
2060, add the jstype = JS_STRING option to the uint64 start_height field
definition inside GetRecentAddressBalanceChangesRequestV0 so it matches other
uint64 block-height fields for JavaScript compatibility; update the field
annotation to include [(gogoproto.jsontag) = "..."] is not needed—simply add the
proto3 field option `jstype = JS_STRING` to the start_height declaration so JS
clients receive the value as a string.

@QuantumExplorer QuantumExplorer changed the title feat: historical storage feat(platform)!: historical address balance storage Dec 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants