Implemented writing on-chain accounts details on block applying (#294)

* feat: add `account_details` table to the DB

* refactor: rename `block_number` column in nullifiers table to `block_num`

* refactor: use `BETWEEN` in interval comparison checks

* feat: implement account details protobuf messages, domain objects and conversions

* feat: (WIP) implement account details support

* feat: (WIP) implement account details support

* feat: (WIP) implement account details support

* feat: (WIP) implement account details support

* fix: db creation

* docs: remove TODO

* refactor: apply formatting

* feat: implement endpoint for getting public account details

* tests: add test for storage

* feat: add rpc endpoint for getting account details

* refactor: keep only domain object changes

* fix: compilation errors

* fix: use note tag conversion from `u64`

* refactor: remove account details protobuf messages

* fix: remove unused error invariants

* refactor: introduce `UpdatedAccount` struct

* fix: rollback details conversion

* fix: compilation error

* feat: account details in store

* refactor: add constraint name for foreign key

* refactor: small code improvement

Co-authored-by: Augusto Hack <hack.augusto@gmail.com>

* feat: account id validation

* refactor: rename `get_account_details` to `select_*`

* feat: return serialized account details

* feat: add requirement of account id to be public in RPC

* fix: remove error message used in different PR

* fix: union account details with account and process them together

* docs: remove `GetAccountDetails` from README.md

* fix: remove unused error invariants

* fix: use `Account` instead of `AccountDetails` in store

* wip

* feat: implement `GetAccountDetails` endpoint

* docs: document `GetAccountDetails` endpoint

* refactor: simplify code, make account details optional

* fix: clippy warning

* fix: address review comments

* fix: update code to the latest miden-base

* refactor: little code improvement

* fix: remove error message used in different PR

* fix: compilation errors
chore: update `miden-base` dependency to fixed version
refactor: extract `apply_delta` function
fix: separate error invariant for missed details in store
fix: make account details optional
refactor: introduce `UpdatedAccount` struct
fix: avoid cloning of block data
feat: simple details validation in store
feat: rewrite `sql::upsert_accounts` to simplified work with details and update test
fix: compilation errors
feat: use serialized account details
feat: writing account details on block applying

* fix: compilation errors, update test

* refactor: rename protobuf messages

* docs: update endpoint in README.md

* tests: get rid of miden-mock dependency

* tests: get rid of winter-rand-utils dependency

* refactor: rename `AccountDetailsUpdate` to `AccountUpdateDetails`

* feat: check for account hash for new on-chain accounts

* formatting: run rustfmt

* docs: address review comments

---------

Co-authored-by: Augusto Hack <hack.augusto@gmail.com>

block-producer/src/batch_builder/batch.rs

@@ -1,11 +1,12 @@
 use std::collections::BTreeMap;
 
-use miden_node_proto::domain::accounts::UpdatedAccount;
+use miden_node_proto::domain::accounts::AccountUpdateDetails;
 use miden_objects::{
     accounts::AccountId,
     batches::BatchNoteTree,
     crypto::hash::blake::{Blake3Digest, Blake3_256},
     notes::{NoteEnvelope, Nullifier},
+    transaction::AccountDetails,
     Digest, MAX_NOTES_PER_BATCH,
 };
 use tracing::instrument;
@@ -54,6 +55,7 @@ impl TransactionBatch {
                     AccountStates {
                         initial_state: tx.initial_account_hash(),
                         final_state: tx.final_account_hash(),
+                        details: tx.account_details().cloned(),
                     },
                 )
             })
@@ -107,15 +109,15 @@ impl TransactionBatch {
             .map(|(account_id, account_states)| (*account_id, account_states.initial_state))
     }
 
-    /// Returns an iterator over (account_id, new_state_hash) tuples for accounts that were
+    /// Returns an iterator over (account_id, details, new_state_hash) tuples for accounts that were
     /// modified in this transaction batch.
-    pub fn updated_accounts(&self) -> impl Iterator<Item = UpdatedAccount> + '_ {
+    pub fn updated_accounts(&self) -> impl Iterator<Item = AccountUpdateDetails> + '_ {
         self.updated_accounts
             .iter()
-            .map(|(&account_id, account_states)| UpdatedAccount {
+            .map(|(&account_id, account_states)| AccountUpdateDetails {
                 account_id,
                 final_state_hash: account_states.final_state,
-                details: None, // TODO: In the next PR: account_states.details.clone(),
+                details: account_states.details.clone(),
             })
     }
 
@@ -150,8 +152,9 @@ impl TransactionBatch {
 /// account.
 ///
 /// TODO: should this be moved into domain objects?
-#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 struct AccountStates {
     initial_state: Digest,
     final_state: Digest,
+    details: Option<AccountDetails>,
 }

block-producer/src/block.rs

@@ -1,7 +1,7 @@
 use std::collections::BTreeMap;
 
 use miden_node_proto::{
-    domain::accounts::UpdatedAccount,
+    domain::accounts::AccountUpdateDetails,
     errors::{ConversionError, MissingFieldHelper},
     generated::responses::GetBlockInputsResponse,
     AccountInputRecord, NullifierWitness,
@@ -18,11 +18,10 @@ use crate::store::BlockInputsError;
 #[derive(Debug, Clone)]
 pub struct Block {
     pub header: BlockHeader,
-    pub updated_accounts: Vec<UpdatedAccount>,
+    pub updated_accounts: Vec<AccountUpdateDetails>,
     pub created_notes: Vec<(usize, usize, NoteEnvelope)>,
     pub produced_nullifiers: Vec<Nullifier>,
     // TODO:
-    // - full states for updated public accounts
     // - full states for created public notes
     // - zk proof
 }

block-producer/src/block_builder/mod.rs

@@ -119,7 +119,7 @@ where
         info!(target: COMPONENT, block_num, %block_hash, "block built");
         debug!(target: COMPONENT, ?block);
 
-        self.state_view.apply_block(block).await?;
+        self.state_view.apply_block(&block).await?;
 
         info!(target: COMPONENT, block_num, %block_hash, "block committed");
 

block-producer/src/block_builder/prover/block_witness.rs

@@ -1,6 +1,6 @@
 use std::collections::{BTreeMap, BTreeSet};
 
-use miden_node_proto::domain::accounts::UpdatedAccount;
+use miden_node_proto::domain::accounts::AccountUpdateDetails;
 use miden_objects::{
     accounts::AccountId,
     crypto::merkle::{EmptySubtreeRoots, MerklePath, MerkleStore, MmrPeaks, SmtProof},
@@ -38,7 +38,7 @@ impl BlockWitness {
 
         let updated_accounts = {
             let mut account_initial_states: BTreeMap<AccountId, Digest> =
-                batches.iter().flat_map(|batch| batch.account_initial_states()).collect();
+                batches.iter().flat_map(TransactionBatch::account_initial_states).collect();
 
             let mut account_merkle_proofs: BTreeMap<AccountId, MerklePath> = block_inputs
                 .accounts
@@ -48,9 +48,9 @@ impl BlockWitness {
 
             batches
                 .iter()
-                .flat_map(|batch| batch.updated_accounts())
+                .flat_map(TransactionBatch::updated_accounts)
                 .map(
-                    |UpdatedAccount {
+                    |AccountUpdateDetails {
                          account_id,
                          final_state_hash,
                          ..

block-producer/src/block_builder/prover/tests.rs

@@ -1,6 +1,6 @@
 use std::{collections::BTreeMap, iter};
 
-use miden_node_proto::domain::accounts::UpdatedAccount;
+use miden_node_proto::domain::accounts::AccountUpdateDetails;
 use miden_objects::{
     accounts::{
         AccountId, ACCOUNT_ID_OFF_CHAIN_SENDER, ACCOUNT_ID_REGULAR_ACCOUNT_UPDATABLE_CODE_OFF_CHAIN,
@@ -239,7 +239,7 @@ async fn test_compute_account_root_success() {
             account_ids
                 .iter()
                 .zip(account_final_states.iter())
-                .map(|(&account_id, &account_hash)| UpdatedAccount {
+                .map(|(&account_id, &account_hash)| AccountUpdateDetails {
                     account_id,
                     final_state_hash: account_hash.into(),
                     details: None,

block-producer/src/state_view/mod.rs

@@ -119,9 +119,9 @@ where
     #[instrument(target = "miden-block-producer", skip_all, err)]
     async fn apply_block(
         &self,
-        block: Block,
+        block: &Block,
     ) -> Result<(), ApplyBlockError> {
-        self.store.apply_block(block.clone()).await?;
+        self.store.apply_block(block).await?;
 
         let mut locked_accounts_in_flight = self.accounts_in_flight.write().await;
         let mut locked_nullifiers_in_flight = self.nullifiers_in_flight.write().await;

block-producer/src/state_view/tests/apply_block.rs

@@ -6,7 +6,7 @@
 
 use std::iter;
 
-use miden_node_proto::domain::accounts::UpdatedAccount;
+use miden_node_proto::domain::accounts::AccountUpdateDetails;
 
 use super::*;
 use crate::test_utils::{block::MockBlockBuilder, MockStoreSuccessBuilder};
@@ -34,7 +34,7 @@ async fn test_apply_block_ab1() {
         .await
         .account_updates(
             std::iter::once(account)
-                .map(|mock_account| UpdatedAccount {
+                .map(|mock_account| AccountUpdateDetails {
                     account_id: mock_account.id,
                     final_state_hash: mock_account.states[1],
                     details: None,
@@ -43,7 +43,7 @@ async fn test_apply_block_ab1() {
         )
         .build();
 
-    let apply_block_res = state_view.apply_block(block).await;
+    let apply_block_res = state_view.apply_block(&block).await;
     assert!(apply_block_res.is_ok());
 
     assert_eq!(*store.num_apply_block_called.read().await, 1);
@@ -81,7 +81,7 @@ async fn test_apply_block_ab2() {
         .account_updates(
             accounts_in_block
                 .into_iter()
-                .map(|mock_account| UpdatedAccount {
+                .map(|mock_account| AccountUpdateDetails {
                     account_id: mock_account.id,
                     final_state_hash: mock_account.states[1],
                     details: None,
@@ -90,7 +90,7 @@ async fn test_apply_block_ab2() {
         )
         .build();
 
-    let apply_block_res = state_view.apply_block(block).await;
+    let apply_block_res = state_view.apply_block(&block).await;
     assert!(apply_block_res.is_ok());
 
     let accounts_still_in_flight = state_view.accounts_in_flight.read().await;
@@ -130,7 +130,7 @@ async fn test_apply_block_ab3() {
             accounts
                 .clone()
                 .into_iter()
-                .map(|mock_account| UpdatedAccount {
+                .map(|mock_account| AccountUpdateDetails {
                     account_id: mock_account.id,
                     final_state_hash: mock_account.states[1],
                     details: None,
@@ -139,7 +139,7 @@ async fn test_apply_block_ab3() {
         )
         .build();
 
-    let apply_block_res = state_view.apply_block(block).await;
+    let apply_block_res = state_view.apply_block(&block).await;
     assert!(apply_block_res.is_ok());
 
     // Craft a new transaction which tries to consume the same note that was consumed in the

block-producer/src/store/mod.rs

@@ -49,7 +49,7 @@ pub trait Store: ApplyBlock {
 pub trait ApplyBlock: Send + Sync + 'static {
     async fn apply_block(
         &self,
-        block: Block,
+        block: &Block,
     ) -> Result<(), ApplyBlockError>;
 }
 
@@ -131,13 +131,13 @@ impl ApplyBlock for DefaultStore {
     #[instrument(target = "miden-block-producer", skip_all, err)]
     async fn apply_block(
         &self,
-        block: Block,
+        block: &Block,
     ) -> Result<(), ApplyBlockError> {
         let request = tonic::Request::new(ApplyBlockRequest {
-            block: Some(block.header.into()),
+            block: Some((&block.header).into()),
             accounts: convert(&block.updated_accounts),
-            nullifiers: convert(block.produced_nullifiers),
-            notes: convert(block.created_notes),
+            nullifiers: convert(&block.produced_nullifiers),
+            notes: convert(&block.created_notes),
         });
 
         let _ = self

block-producer/src/test_utils/block.rs

@@ -1,4 +1,4 @@
-use miden_node_proto::domain::accounts::UpdatedAccount;
+use miden_node_proto::domain::accounts::AccountUpdateDetails;
 use miden_objects::{
     block::BlockNoteTree,
     crypto::merkle::{Mmr, SimpleSmt},
@@ -68,7 +68,7 @@ pub async fn build_actual_block_header(
     let updated_accounts: Vec<_> =
         batches.iter().flat_map(TransactionBatch::updated_accounts).collect();
     let produced_nullifiers: Vec<Nullifier> =
-        batches.iter().flat_map(|batch| batch.produced_nullifiers()).collect();
+        batches.iter().flat_map(TransactionBatch::produced_nullifiers).collect();
 
     let block_inputs_from_store: BlockInputs = store
         .get_block_inputs(
@@ -89,7 +89,7 @@ pub struct MockBlockBuilder {
     store_chain_mmr: Mmr,
     last_block_header: BlockHeader,
 
-    updated_accounts: Option<Vec<UpdatedAccount>>,
+    updated_accounts: Option<Vec<AccountUpdateDetails>>,
     created_notes: Option<Vec<(usize, usize, NoteEnvelope)>>,
     produced_nullifiers: Option<Vec<Nullifier>>,
 }
@@ -109,7 +109,7 @@ impl MockBlockBuilder {
 
     pub fn account_updates(
         mut self,
-        updated_accounts: Vec<UpdatedAccount>,
+        updated_accounts: Vec<AccountUpdateDetails>,
     ) -> Self {
         for update in &updated_accounts {
             self.store_accounts

block-producer/src/test_utils/store.rs

@@ -174,7 +174,7 @@ impl MockStoreSuccess {
 impl ApplyBlock for MockStoreSuccess {
     async fn apply_block(
         &self,
-        block: Block,
+        block: &Block,
     ) -> Result<(), ApplyBlockError> {
         // Intentionally, we take and hold both locks, to prevent calls to `get_tx_inputs()` from going through while we're updating the store's data structure
         let mut locked_accounts = self.accounts.write().await;
@@ -187,7 +187,7 @@ impl ApplyBlock for MockStoreSuccess {
         debug_assert_eq!(locked_accounts.root(), block.header.account_root());
 
         // update nullifiers
-        for nullifier in block.produced_nullifiers {
+        for nullifier in &block.produced_nullifiers {
             locked_produced_nullifiers
                 .insert(nullifier.inner(), [block.header.block_num().into(), ZERO, ZERO, ZERO]);
         }
@@ -291,7 +291,7 @@ pub struct MockStoreFailure;
 impl ApplyBlock for MockStoreFailure {
     async fn apply_block(
         &self,
-        _block: Block,
+        _block: &Block,
     ) -> Result<(), ApplyBlockError> {
         Err(ApplyBlockError::GrpcClientError(String::new()))
     }

proto/proto/account.proto

@@ -10,13 +10,13 @@ message AccountId {
     fixed64 id = 1;
 }
 
-message AccountHashUpdate {
-    account.AccountId account_id = 1;
+message AccountSummary {
+    AccountId account_id = 1;
     digest.Digest account_hash = 2;
     uint32 block_num = 3;
 }
 
 message AccountInfo {
-    AccountHashUpdate update = 1;
+    AccountSummary summary = 1;
     optional bytes details = 2;
 }

proto/proto/requests.proto

@@ -10,6 +10,8 @@ import "note.proto";
 message AccountUpdate {
     account.AccountId account_id = 1;
     digest.Digest account_hash = 2;
+    // Details for public (on-chain) account.
+    optional bytes details = 3;
 }
 
 message ApplyBlockRequest {

proto/proto/responses.proto

@@ -36,7 +36,7 @@ message SyncStateResponse {
     mmr.MmrDelta mmr_delta = 3;
 
     // a list of account hashes updated after `block_num + 1` but not after `block_header.block_num`
-    repeated account.AccountHashUpdate accounts = 5;
+    repeated account.AccountSummary accounts = 5;
 
     // a list of all notes together with the Merkle paths from `block_header.note_root`
     repeated note.NoteSyncRecord notes = 6;