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

(Logs) Submit proven transaction logs #172

Merged
merged 29 commits into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
7c78c39
feat(logs): added logs for `submit_proven_transaction` flow
polydez Jan 22, 2024
d95e3db
fix(logs): implemented `Debug` for `TxInputs`
polydez Jan 22, 2024
482e0cc
feat(logs): additional logging and logging improvements
polydez Jan 22, 2024
58b85a9
fix(logs): removed unnecessary log data
polydez Jan 23, 2024
9bb8583
feat(logs): improved formatting, reduced verbosity
polydez Jan 23, 2024
f97efdb
feat(logs): improved formatting, reduced verbosity
polydez Jan 23, 2024
1857754
feat(logs): improved formatting, reduced verbosity
polydez Jan 23, 2024
9a14ade
fix(logs): improved work with `COMPONENT`
polydez Jan 24, 2024
765496e
fix(logs): using `target` instead of `COMPONENT`
polydez Jan 24, 2024
c19c0a0
fix(logs): reduced verbosity
polydez Jan 24, 2024
58d738b
fix(logs): implemented `Debug` and `Display` for protobuf's `AccountId`
polydez Jan 25, 2024
8e77403
feat(logs): thin wrapper for `AccountId(u64)`, improved formatting of…
polydez Jan 25, 2024
f0ded9f
feat(logs): improved formatting of `AccountState` and `NullifierState…
polydez Jan 25, 2024
710118a
Merge remote-tracking branch 'origin/main' into polydez-logs-submit-p…
polydez Jan 25, 2024
0610251
feat(logs): removed `num_assets` from output notes formatting
polydez Jan 25, 2024
3414bac
fix(logs): improved formatting, implemented `Display` stubs for errors
polydez Jan 25, 2024
44cbdbb
Revert "feat(logs): thin wrapper for `AccountId(u64)`, improved forma…
polydez Jan 25, 2024
ea1acf2
fix(logs) reverted changes from other flows
polydez Jan 25, 2024
193d4c3
fix(logs) removed `target!()` macro, using `COMPONENT` as target
polydez Jan 25, 2024
07a0231
fix(logs) removed all fields from spans
polydez Jan 25, 2024
91162e6
fix(logs) override names of API handlers
polydez Jan 25, 2024
1e34f5b
fix(logs) addressing review comments
polydez Jan 25, 2024
2c076a2
fix(logs) warnings
polydez Jan 25, 2024
0b3ba55
fix(logs) addressed review comments
polydez Jan 26, 2024
36383b6
Merge remote-tracking branch 'origin/main' into polydez-logs-submit-p…
polydez Jan 26, 2024
0239c1e
fix(logs) `skip_all` for `State::load`
polydez Jan 26, 2024
09197be
fix(logs) small code improvement
polydez Jan 26, 2024
49c4022
fix(logs) addressing review comments
polydez Jan 26, 2024
a7939c4
Merge remote-tracking branch 'origin/main' into polydez-logs-submit-p…
polydez Jan 26, 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
4 changes: 2 additions & 2 deletions block-producer/src/batch_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use tokio::{sync::RwLock, time};
use tracing::info;

use self::errors::BuildBatchError;
use crate::{block_builder::BlockBuilder, SharedProvenTx, SharedRwVec, SharedTxBatch, COMPONENT};
use crate::{block_builder::BlockBuilder, target, SharedProvenTx, SharedRwVec, SharedTxBatch};

pub mod errors;
#[cfg(test)]
Expand Down Expand Up @@ -120,7 +120,7 @@ where
let batch = Arc::new(TransactionBatch::new(txs)?);
self.ready_batches.write().await.push(batch);

info!(COMPONENT, "batch built with {num_txs} txs");
info!(target: target!(), num_txs, "batch built");

Ok(())
}
Expand Down
4 changes: 2 additions & 2 deletions block-producer/src/block_builder/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use tracing::info;
use crate::{
block::Block,
store::{ApplyBlock, Store},
SharedTxBatch, COMPONENT, MAX_NUM_CREATED_NOTES_PER_BATCH,
target, SharedTxBatch, MAX_NUM_CREATED_NOTES_PER_BATCH,
};

pub mod errors;
Expand Down Expand Up @@ -110,7 +110,7 @@ where

self.state_view.apply_block(block).await?;

info!(COMPONENT, "block #{block_num} built!");
info!(target: target!(), block_num, "block is built!");

Ok(())
}
Expand Down
10 changes: 7 additions & 3 deletions block-producer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,6 @@ pub(crate) type SharedRwVec<T> = Arc<RwLock<Vec<T>>>;
// CONSTANTS
// =================================================================================================

/// The name of the block producer component
pub const COMPONENT: &str = "miden-block-producer";

/// The depth of the SMT for created notes
const CREATED_NOTES_SMT_DEPTH: u8 = 13;

Expand All @@ -53,3 +50,10 @@ const SERVER_BUILD_BATCH_FREQUENCY: Duration = Duration::from_secs(2);

/// Maximum number of batches per block
const SERVER_MAX_BATCHES_PER_BLOCK: usize = 4;

#[macro_export]
macro_rules! target {
() => {
"miden-block-producer"
};
}
20 changes: 19 additions & 1 deletion block-producer/src/server/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ use miden_node_proto::{
block_producer::api_server, requests::SubmitProvenTransactionRequest,
responses::SubmitProvenTransactionResponse,
};
use miden_node_utils::logging::{format_input_notes, format_opt, format_output_notes};
use miden_objects::transaction::ProvenTransaction;
use tonic::Status;
use tracing::{debug, info, instrument};

use crate::txqueue::TransactionQueue;
use crate::{target, txqueue::TransactionQueue};

// BLOCK PRODUCER
// ================================================================================================
Expand All @@ -32,15 +34,31 @@ impl<T> api_server::Api for BlockProducerApi<T>
where
T: TransactionQueue,
{
#[allow(clippy::blocks_in_conditions)] // Workaround of `instrument` issue
#[instrument(target = "miden-block-producer", skip(self, request), err)]
async fn submit_proven_transaction(
bobbinth marked this conversation as resolved.
Show resolved Hide resolved
&self,
request: tonic::Request<SubmitProvenTransactionRequest>,
) -> Result<tonic::Response<SubmitProvenTransactionResponse>, Status> {
let request = request.into_inner();
debug!(target: target!(), tx = ?request.transaction);
bobbinth marked this conversation as resolved.
Show resolved Hide resolved

let tx = ProvenTransaction::read_from_bytes(&request.transaction)
.map_err(|_| Status::invalid_argument("Invalid transaction"))?;

info!(
target: target!(),
tx_id = %tx.id().inner(),
account_id = %tx.account_id(),
bobbinth marked this conversation as resolved.
Show resolved Hide resolved
initial_account_hash = %tx.initial_account_hash(),
final_account_hash = %tx.final_account_hash(),
input_notes = %format_input_notes(tx.input_notes()),
output_notes = %format_output_notes(tx.output_notes()),
tx_script_root = %format_opt(tx.tx_script_root().as_ref()),
block_ref = %tx.block_ref(),
);
debug!(target: target!(), proof = ?tx.proof());

self.queue
.add_transaction(Arc::new(tx))
.await
Expand Down
28 changes: 16 additions & 12 deletions block-producer/src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,17 @@ use std::{net::ToSocketAddrs, sync::Arc};
use anyhow::Result;
use miden_node_proto::{block_producer::api_server, store::api_client as store_client};
use tonic::transport::Server;
use tracing::info;
use tracing::{info, info_span, instrument, Instrument};

use crate::{
batch_builder::{DefaultBatchBuilder, DefaultBatchBuilderOptions},
block_builder::DefaultBlockBuilder,
config::BlockProducerConfig,
state_view::DefaultStateView,
store::DefaultStore,
target,
txqueue::{DefaultTransactionQueue, DefaultTransactionQueueOptions},
COMPONENT, SERVER_BATCH_SIZE, SERVER_BLOCK_FREQUENCY, SERVER_BUILD_BATCH_FREQUENCY,
SERVER_BATCH_SIZE, SERVER_BLOCK_FREQUENCY, SERVER_BUILD_BATCH_FREQUENCY,
SERVER_MAX_BATCHES_PER_BLOCK,
};

Expand All @@ -23,7 +24,10 @@ pub mod api;
// ================================================================================================

/// TODO: add comments
#[instrument(target = "miden-block-producer", skip(config))]
pub async fn serve(config: BlockProducerConfig) -> Result<()> {
bobbinth marked this conversation as resolved.
Show resolved Hide resolved
info!(target: target!(), ?config);

let endpoint = (config.endpoint.host.as_ref(), config.endpoint.port);
let addrs: Vec<_> = endpoint.to_socket_addrs()?.collect();

Expand Down Expand Up @@ -53,21 +57,21 @@ pub async fn serve(config: BlockProducerConfig) -> Result<()> {
let block_producer = api_server::ApiServer::new(api::BlockProducerApi::new(queue.clone()));

tokio::spawn(async move {
info!(COMPONENT, "transaction queue started");
queue.run().await
queue
.run()
.instrument(info_span!(target: target!(), "transaction_queue_start"))
.await
});

tokio::spawn(async move {
info!(COMPONENT, "batch builder started");
batch_builder.run().await
batch_builder
.run()
.instrument(info_span!(target: target!(), "batch_builder_start"))
.await
});

info!(
COMPONENT,
host = config.endpoint.host,
port = config.endpoint.port,
"Server initialized",
);
info!(target: target!(), host = config.endpoint.host, port = config.endpoint.port, "Server initialized");

Server::builder().add_service(block_producer).serve(addrs[0]).await?;

Ok(())
Expand Down
11 changes: 11 additions & 0 deletions block-producer/src/state_view/mod.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
use std::{collections::BTreeSet, sync::Arc};

use async_trait::async_trait;
use miden_node_utils::logging::format_opt;
use miden_objects::{accounts::AccountId, notes::Nullifier, transaction::InputNotes, Digest};
use tokio::sync::RwLock;
use tracing::instrument;

use crate::{
block::Block,
Expand Down Expand Up @@ -44,6 +46,8 @@ where
S: Store,
{
// TODO: Verify proof as well
#[allow(clippy::blocks_in_conditions)] // Workaround of `instrument` issue
#[instrument(skip(self, candidate_tx), err(Debug), fields(account_id = %candidate_tx.account_id().to_hex()))]
async fn verify_tx(
&self,
candidate_tx: SharedProvenTx,
Expand Down Expand Up @@ -131,6 +135,7 @@ where
/// 1. the candidate transaction doesn't modify the same account as an existing in-flight transaction
/// 2. no consumed note's nullifier in candidate tx's consumed notes is already contained
/// in `already_consumed_nullifiers`
#[instrument(target = "miden-block-producer", skip(candidate_tx), err(Debug))]
fn ensure_in_flight_constraints(
candidate_tx: SharedProvenTx,
accounts_in_flight: &BTreeSet<AccountId>,
Expand Down Expand Up @@ -162,6 +167,12 @@ fn ensure_in_flight_constraints(
Ok(())
}

#[instrument(
target = "miden-block-producer",
skip(candidate_tx, tx_inputs),
err(Debug),
fields(tx_inputs.account_hash = %format_opt(tx_inputs.account_hash.as_ref()), nullifiers = ?tx_inputs.nullifiers))
bobbinth marked this conversation as resolved.
Show resolved Hide resolved
]
fn ensure_tx_inputs_constraints(
candidate_tx: SharedProvenTx,
tx_inputs: TxInputs,
Expand Down
23 changes: 20 additions & 3 deletions block-producer/src/store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ use miden_node_proto::{
};
use miden_objects::{accounts::AccountId, Digest};
use tonic::transport::Channel;
use tracing::{info, instrument};

use crate::{block::Block, SharedProvenTx};
use crate::{block::Block, target, SharedProvenTx};

mod errors;
pub use errors::{ApplyBlockError, BlockInputsError, TxInputsError};
use miden_node_utils::logging::format_opt;

// STORE TRAIT
// ================================================================================================
Expand Down Expand Up @@ -45,6 +47,7 @@ pub trait ApplyBlock: Send + Sync + 'static {
}

/// Information needed from the store to verify a transaction.
#[derive(Debug)]
pub struct TxInputs {
/// The account hash in the store corresponding to tx's account ID
pub account_hash: Option<Digest>,
Expand Down Expand Up @@ -93,18 +96,24 @@ impl ApplyBlock for DefaultStore {

#[async_trait]
impl Store for DefaultStore {
#[allow(clippy::blocks_in_conditions)] // Workaround of `instrument` issue
#[instrument(target = "miden-block-producer", skip(self, proven_tx), err)]
async fn get_tx_inputs(
&self,
proven_tx: SharedProvenTx,
) -> Result<TxInputs, TxInputsError> {
let request = tonic::Request::new(GetTransactionInputsRequest {
let message = GetTransactionInputsRequest {
account_id: Some(proven_tx.account_id().into()),
nullifiers: proven_tx
.input_notes()
.iter()
.map(|nullifier| (*nullifier).into())
.collect(),
});
};

info!(target: target!(), ?message);
bobbinth marked this conversation as resolved.
Show resolved Hide resolved

let request = tonic::Request::new(message);
let response = self
.store
.clone()
Expand All @@ -113,6 +122,8 @@ impl Store for DefaultStore {
.map_err(|status| TxInputsError::GrpcClientError(status.message().to_string()))?
.into_inner();

info!(target: target!(), ?response);

let account_hash = {
let account_state = response
.account_state
Expand Down Expand Up @@ -153,6 +164,12 @@ impl Store for DefaultStore {
nullifiers.into_iter().collect()
};

info!(
target: target!(),
account_hash = %format_opt(account_hash.as_ref()),
?nullifiers,
);
bobbinth marked this conversation as resolved.
Show resolved Hide resolved

Ok(TxInputs {
account_hash,
nullifiers,
Expand Down
13 changes: 11 additions & 2 deletions block-producer/src/txqueue/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@ use miden_objects::{
accounts::AccountId, notes::Nullifier, transaction::InputNotes, Digest, TransactionInputError,
};
use tokio::{sync::RwLock, time};
use tracing::{info_span, instrument, Instrument};

use crate::{batch_builder::BatchBuilder, store::TxInputsError, SharedProvenTx, SharedRwVec};
use crate::{
batch_builder::BatchBuilder, store::TxInputsError, target, SharedProvenTx, SharedRwVec,
};

#[cfg(test)]
mod tests;
Expand Down Expand Up @@ -138,7 +141,11 @@ where
let batch_builder = self.batch_builder.clone();

tokio::spawn(async move {
match batch_builder.build_batch(txs.clone()).await {
match batch_builder
.build_batch(txs.clone())
.instrument(info_span!(target: target!(), "batch_builder::build_batch"))
.await
{
Ok(_) => {
// batch was successfully built, do nothing
},
Expand All @@ -158,6 +165,8 @@ where
TV: TransactionVerifier,
BB: BatchBuilder,
{
#[allow(clippy::blocks_in_conditions)] // Workaround of `instrument` issue
#[instrument(target = "miden-block-producer", skip(self, tx), err(Debug), fields(tx_id = %tx.id().inner()))]
async fn add_transaction(
&self,
tx: SharedProvenTx,
Expand Down
2 changes: 2 additions & 0 deletions node/src/commands/genesis/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use miden_objects::{
assets::TokenSymbol,
Felt,
};
use tracing::instrument;

mod inputs;

Expand All @@ -41,6 +42,7 @@ const DEFAULT_ACCOUNTS_DIR: &str = "accounts/";
/// This function returns a `Result` type. On successful creation of the genesis file, it returns
/// `Ok(())`. If it fails at any point, due to issues like file existence checks or read/write
/// operations, it returns an `Err` with a detailed error message.
#[instrument(target = "miden-node")]
pub fn make_genesis(
inputs_path: &PathBuf,
output_path: &PathBuf,
Expand Down
2 changes: 2 additions & 0 deletions node/src/commands/start.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use miden_node_store::{config::StoreConfig, db::Db, server as store_server};
use miden_node_utils::config::load_config;
use serde::{Deserialize, Serialize};
use tokio::task::JoinSet;
use tracing::instrument;

// Top-level config
// ================================================================================================
Expand All @@ -22,6 +23,7 @@ pub struct StartCommandConfig {
// START
// ===================================================================================================

#[instrument(target = "miden-node")]
pub async fn start_node(config_filepath: &Path) -> Result<()> {
let config: StartCommandConfig = load_config(config_filepath).extract().map_err(|err| {
anyhow!("failed to load config file `{}`: {err}", config_filepath.display())
Expand Down
1 change: 1 addition & 0 deletions proto/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ proptest = { version = "1.2" }
[build-dependencies]
miette = { version = "5.9", features = ["fancy"] }
prost = { version = "0.12" }
prost-build = { version = "0.12" }
protox = { version = "0.5" }
tonic-build = { version = "0.10" }
5 changes: 4 additions & 1 deletion proto/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,16 @@ fn main() -> miette::Result<()> {
let file_descriptors = protox::compile(protos, includes)?;
fs::write(&file_descriptor_path, file_descriptors.encode_to_vec()).into_diagnostic()?;

let mut prost_config = prost_build::Config::new();
prost_config.skip_debug(["Digest"]);

// Generate the stub of the user facing server from its proto file
tonic_build::configure()
.file_descriptor_set_path(&file_descriptor_path)
.type_attribute(".", "#[derive(Eq, PartialOrd, Ord, Hash)]")
.skip_protoc_run()
.out_dir("src/generated")
.compile(protos, includes)
.compile_with_config(prost_config, protos, includes)
.into_diagnostic()?;

Ok(())
Expand Down
1 change: 1 addition & 0 deletions proto/src/generated/digest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#[derive(Eq, PartialOrd, Ord, Hash)]
#[allow(clippy::derive_partial_eq_without_eq)]
#[derive(Clone, PartialEq, ::prost::Message)]
#[prost(skip_debug)]
pub struct Digest {
#[prost(fixed64, tag = "1")]
pub d0: u64,
Expand Down
Loading
Loading