Connect 4 nodes in block-producer, and create miden-node binary
#85
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
plafer
changed the title
miden-node binary
hackaugusto
reviewed
Dec 8, 2023
block-producer/src/server/api.rs
Outdated
Comment on lines
50
to
53
| .map(|(account_id, account_hash)| AccountUpdate { | ||
| account_id: Some((*account_id).into()), | ||
| account_hash: Some(account_hash.into()), | ||
| }) |
There was a problem hiding this comment.
suggestions:
- add
impl From<Account> for AccountUpdateas a conversion. - use
convert
should make the code a bit cleaner
There was a problem hiding this comment.
changed all of them to use convert, and created #99 to address the clones
Comment on lines
+118
to
+134
| let nullifiers = { | ||
| let mut nullifiers = Vec::new(); | ||
|
|
||
| for nullifier_record in response.nullifiers { | ||
| let nullifier = nullifier_record | ||
| .nullifier | ||
| .ok_or(TxInputsError::MalformedResponse( | ||
| "nullifier record contains empty nullifier".to_string(), | ||
| ))? | ||
| .try_into()?; | ||
|
|
||
| // `block_num` is nonzero if already consumed; 0 otherwise | ||
| nullifiers.push((nullifier, nullifier_record.block_num != 0)) | ||
| } | ||
|
|
||
| nullifiers.into_iter().collect() | ||
| }; |
There was a problem hiding this comment.
maybe this can be replaced with try_convert?
There was a problem hiding this comment.
I cannot use try_convert because I cannot implement TryFrom<NullifierTransactionInputRecord> for (RpoDigest, bool) where Error = TxInputsError, since TxInputsError lives in miden-node-block-producer (and our conversion code lives in miden-node-proto).
Related: #74 (comment)
hackaugusto
force-pushed
the
plafer-start-node
branch
from
c6cc90c to
7b1e2e3
Compare
This reverts commit 38c19ce.
`deadpool`'s method `interact` is blocking, it waits for the closure to complete its execution before returning. This causes a deadlock, because of the way state synchronization is performed in the store server. In the store, the transaction is opened and data is written to the WAL file, but not immedially committed. Before comiting the data, the in-memory datastructures prevent read operations. This meanst the server has to acquire the exclusive write lock. This operation demands concurrency and synchronization among the DB and state components. This required a change from a blocking to a concurrent API. `deadpool` doesn't seem to have a native concurrent API, so a async task was used instead. `tokio` does not support scoped tasks, so thread local state can not be used across tasks, for this reason all the data was modified from being borrowed to being owned, so that the data can be moved in to the async task.
…ode into plafer-start-node
bobbinth
requested changes
Dec 12, 2023
This was referenced Dec 12, 2023
bobbinth
approved these changes
Dec 12, 2023
block-producer/src/lib.rs
Outdated
Comment on lines
35
to
56
| /// The depth of the SMT for created notes | ||
| pub(crate) const CREATED_NOTES_SMT_DEPTH: u8 = 13; | ||
|
|
||
| /// The maximum number of created notes per batch. | ||
| /// | ||
| /// The created notes tree uses an extra depth to store the 2 components of `NoteEnvelope`. | ||
| /// That is, conceptually, notes sit at depth 12; where in reality, depth 12 contains the | ||
| /// hash of level 13, where both the `note_hash()` and metadata are stored (one per node). | ||
| pub(crate) const MAX_NUM_CREATED_NOTES_PER_BATCH: usize = | ||
| 2_usize.pow((CREATED_NOTES_SMT_DEPTH - 1) as u32); | ||
|
|
||
| /// The number of transactions per batch | ||
| pub(crate) const SERVER_BATCH_SIZE: usize = 2; | ||
|
|
||
| /// The frequency at which blocks are produced | ||
| pub(crate) const SERVER_BLOCK_FREQUENCY: Duration = Duration::from_secs(10); | ||
|
|
||
| /// The frequency at which batches are built | ||
| pub(crate) const SERVER_BUILD_BATCH_FREQUENCY: Duration = Duration::from_secs(2); | ||
|
|
||
| /// Maximum number of batches per block | ||
| pub(crate) const SERVER_MAX_BATCHES_PER_BLOCK: usize = 4; |
There was a problem hiding this comment.
nit: since these are now in lib.rs, I think we should be able to get rid of pub(crate) modifier.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #17
Closes #42
Noteworthy changes/decisions:
BlockHeaderas the "genesis header"