Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 60 commits into from
Apr 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
b6c529f
feat: add `account_details` table to the DB
polydez Mar 8, 2024
ed04d5a
refactor: rename `block_number` column in nullifiers table to `block_…
polydez Mar 8, 2024
4cc4ba8
refactor: use `BETWEEN` in interval comparison checks
polydez Mar 8, 2024
8f0935a
feat: implement account details protobuf messages, domain objects and…
polydez Mar 10, 2024
9f5c832
feat: (WIP) implement account details support
polydez Mar 10, 2024
58dc518
feat: (WIP) implement account details support
polydez Mar 12, 2024
ef7a1f7
feat: (WIP) implement account details support
polydez Mar 12, 2024
bb31753
feat: (WIP) implement account details support
polydez Mar 25, 2024
ac81502
fix: db creation
polydez Mar 25, 2024
ab9c457
docs: remove TODO
polydez Mar 26, 2024
31cfd19
refactor: apply formatting
polydez Mar 26, 2024
6560444
feat: implement endpoint for getting public account details
polydez Mar 27, 2024
bd7ac74
Merge branch 'next' into polydez-onchain-accounts
polydez Mar 28, 2024
be2be66
tests: add test for storage
polydez Mar 29, 2024
fbcfd6b
feat: add rpc endpoint for getting account details
polydez Mar 29, 2024
28d44fb
refactor: keep only domain object changes
polydez Mar 29, 2024
d344c57
Merge branch 'next' into polydez-onchain-accounts
polydez Mar 29, 2024
618e24e
fix: compilation errors
polydez Mar 30, 2024
bd47398
fix: use note tag conversion from `u64`
polydez Apr 1, 2024
42b0c22
refactor: remove account details protobuf messages
polydez Apr 3, 2024
bb6e18e
fix: remove unused error invariants
polydez Apr 3, 2024
535797e
refactor: introduce `UpdatedAccount` struct
polydez Apr 5, 2024
3266818
fix: rollback details conversion
polydez Apr 5, 2024
fcdc331
fix: compilation error
polydez Apr 5, 2024
6c9b79f
feat: account details in store
polydez Mar 30, 2024
93e792c
refactor: add constraint name for foreign key
polydez Apr 1, 2024
8cc29f5
refactor: small code improvement
polydez Apr 1, 2024
69c516f
feat: account id validation
polydez Apr 1, 2024
ed9a200
refactor: rename `get_account_details` to `select_*`
polydez Apr 3, 2024
55a3933
feat: return serialized account details
polydez Apr 3, 2024
1b4d0da
feat: add requirement of account id to be public in RPC
polydez Apr 3, 2024
1e00839
fix: remove error message used in different PR
polydez Apr 3, 2024
b790e33
fix: union account details with account and process them together
polydez Apr 4, 2024
85bf4d0
docs: remove `GetAccountDetails` from README.md
polydez Apr 4, 2024
f677ce0
fix: remove unused error invariants
polydez Apr 4, 2024
8275a89
fix: use `Account` instead of `AccountDetails` in store
polydez Apr 4, 2024
78f29be
wip
polydez Apr 5, 2024
12a8bb9
Merge branch 'next' into polydez-onchain-accounts-store
polydez Apr 5, 2024
09987a8
Merge branch 'next' into polydez-onchain-accounts-store
polydez Apr 5, 2024
9fc3f60
feat: implement `GetAccountDetails` endpoint
polydez Apr 5, 2024
6463f71
docs: document `GetAccountDetails` endpoint
polydez Apr 5, 2024
1652048
refactor: simplify code, make account details optional
polydez Apr 5, 2024
49f5d12
fix: clippy warning
polydez Apr 5, 2024
da0deed
fix: address review comments
polydez Apr 6, 2024
fb7d577
fix: update code to the latest miden-base
polydez Apr 6, 2024
1e3e713
Merge branch 'next' into polydez-onchain-accounts-store
polydez Apr 6, 2024
955be92
refactor: little code improvement
polydez Apr 6, 2024
dc2a8bc
fix: remove error message used in different PR
polydez Apr 3, 2024
df3d2df
fix: compilation errors
polydez Mar 30, 2024
22c89bb
Merge branch 'next' into polydez-onchain-accounts-apply
polydez Apr 6, 2024
328042b
fix: compilation errors, update test
polydez Apr 6, 2024
65caccc
refactor: rename protobuf messages
polydez Apr 8, 2024
6b5c313
docs: update endpoint in README.md
polydez Apr 8, 2024
895b982
tests: get rid of miden-mock dependency
polydez Apr 8, 2024
6aa4819
tests: get rid of winter-rand-utils dependency
polydez Apr 8, 2024
d69af1d
refactor: rename `AccountDetailsUpdate` to `AccountUpdateDetails`
polydez Apr 8, 2024
bd4355c
feat: check for account hash for new on-chain accounts
polydez Apr 8, 2024
4af5946
Merge branch 'next' into polydez-onchain-accounts-apply
polydez Apr 9, 2024
652905a
formatting: run rustfmt
polydez Apr 9, 2024
609aaef
docs: address review comments
polydez Apr 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions block-producer/src/batch_builder/batch.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -54,6 +55,7 @@ impl TransactionBatch {
AccountStates {
initial_state: tx.initial_account_hash(),
final_state: tx.final_account_hash(),
details: tx.account_details().cloned(),
},
)
})
Expand Down Expand Up @@ -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(),
})
}

Expand Down Expand Up @@ -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>,
}
5 changes: 2 additions & 3 deletions block-producer/src/block.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion block-producer/src/block_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down
8 changes: 4 additions & 4 deletions block-producer/src/block_builder/prover/block_witness.rs
Original file line number Diff line number Diff line change
@@ -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},
Expand Down Expand Up @@ -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
Expand All @@ -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,
..
Expand Down
4 changes: 2 additions & 2 deletions block-producer/src/block_builder/prover/tests.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions block-producer/src/state_view/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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?;
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume that store.apply_block encodes the Block into a protobuf message. Meaning it doesn't need ownership of Block. If that is the case, I think we should change it to receive a reference, and then here we don't need the copies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hackaugusto we can't do it simple way because the block needs be disassembled inside apply_block:

block: Some(block.header.into()),
accounts: convert(block.updated_accounts),
nullifiers: convert(block.produced_nullifiers),
notes: convert(block.created_notes),

Copy link
Contributor

Choose a reason for hiding this comment

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

The linked code is creating new objects. At first sight it seems none of them require ownership.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks for noticing!


let mut locked_accounts_in_flight = self.accounts_in_flight.write().await;
let mut locked_nullifiers_in_flight = self.nullifiers_in_flight.write().await;
Expand Down
14 changes: 7 additions & 7 deletions block-producer/src/state_view/tests/apply_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
Expand All @@ -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);
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand Down
10 changes: 5 additions & 5 deletions block-producer/src/store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
}

Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions block-producer/src/test_utils/block.rs
Original file line number Diff line number Diff line change
@@ -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},
Expand Down Expand Up @@ -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(
Expand All @@ -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>>,
}
Expand All @@ -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
Expand Down
6 changes: 3 additions & 3 deletions block-producer/src/test_utils/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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]);
}
Expand Down Expand Up @@ -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()))
}
Expand Down
6 changes: 3 additions & 3 deletions proto/proto/account.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
2 changes: 2 additions & 0 deletions proto/proto/requests.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion proto/proto/responses.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading
Loading