From 6c16dd137f945fb991b09f565148e8dcf83490cf Mon Sep 17 00:00:00 2001 From: Josef Widder <44643235+josef-widder@users.noreply.github.com> Date: Fri, 22 Nov 2024 14:10:50 +0100 Subject: [PATCH 1/4] fix(spec): rewrite of English blocksync spec (#589) * start of rewrite * spec: BlockSync renamed to Blocksync * spec/blockchain: minor fixes * analysis doc * Apply suggestions from code review Co-authored-by: Daniel --------- Co-authored-by: Daniel Cason --- specs/english/synchronization/README.md | 83 ++++++++++++++++--------- specs/quint/specs/blocksync/README.md | 22 +++++++ 2 files changed, 75 insertions(+), 30 deletions(-) create mode 100644 specs/quint/specs/blocksync/README.md diff --git a/specs/english/synchronization/README.md b/specs/english/synchronization/README.md index 54a1b91f7..48c02a84a 100644 --- a/specs/english/synchronization/README.md +++ b/specs/english/synchronization/README.md @@ -1,15 +1,15 @@ -# Malachite Synchronization Protocol MVP Specification +# Malachite Blocksync Protocol MVP Specification -The challenge we address with the synchronization protocol is long-term stability: +The challenge we address with Blocksync is long-term stability: When running for a long time, the consensus mechanism alone may not be enough to keep the system alive. If nodes are down or disconnected for some (extended period of) time, they may fall behind several heights. -While to consensus algorithm is fault-tolerant, if too many nodes fall behind, and are not able to catch up, eventually we might not have sufficiently many validators synchronized at the top of the chain to make progress. +While the consensus algorithm is fault-tolerant, if too many nodes fall behind, and are not able to catch up, eventually we might not have sufficiently many validators synchronized at the top of the chain to make progress. We consider a composition of three components - Consensus. The consensus node: Executing consensus iteratively over multiple heights, and storing the decided blocks -- Client. Synchronization Client: That tries to obtain data (certificates, blocks) in order to decide quickly, in case the node has fallen behind, and other nodes have already decided on blocks. -- Server. Synchronization Server. Provides data about decided blocks to clients +- Client. Blocksync Client: That tries to obtain data (certificates, blocks) in order to decide quickly, in case the node has fallen behind and other nodes have already decided on blocks. +- Server. Blocksync Server. Provides data about decided blocks to clients. #### Outline of the protocol @@ -18,29 +18,33 @@ The rough idea of the protocol is the following - Consensus, client and server run in parallel on a node - The client observes the height of its local consensus instance - The server regularly announces the height of its local consensus instance to the network -- When a client observes that the local height is smaller than a remote height, it requests for a missing height: the commit (a certificate consisting of 2f+1 precommit messages) and the committed block (proposal) +- When a client observes that the local height is smaller than a remote height, it requests a missing height: the commit (a certificate consisting of 2f+1 Precommit messages) and the committed block (proposal) - When a server receives such a request, it obtains the required information from the local block store, and sends it to the client -- When a client receives a response (certificate or proposal), it delivers this information to the consensus logic +- When a client receives a response (certificate or proposal or both), it delivers this information to the consensus logic - The consensus logic (driver) handles this incoming information in the same way as it would handle it if it came from "normal" consensus operation. ## Design decision Observe that in the protocol description above, we have already taken a big design decision, namely, that the components consensus, client, and server run in parallel. This has been a conscious decision. -There are other potential designs, where the protocols alternate: when a node figures out that it has fallen behind several heights, it switches from consensus mode to synchronization mode, and then, when it has finished synchronizing, it switches back to consensus. This approach has two downsides (1) consensus blocks are decided in (at least) two locations of the code, consensus and synchronization, and (2) what are the precise conditions to switch between the two modes, and what are the expectations about the local state (e.g., incoming message buffers, etc.) when such a switch happens? Particularly Point 2 is quite hard to get right in a distributed setting. For instance, a node might locally believe that it is synchronized, while others have actually moved ahead instead. This creates a lot of algorithmic complexity as well as complications in the analysis of the protocols. +There are other potential designs, where the protocols alternate: when a node figures out that it has fallen behind several heights, it switches from consensus mode to synchronization mode, and then, when it has finished synchronizing, it switches back to consensus. +This approach has two downsides: (1) consensus blocks are decided in (at least) two locations of the code, consensus and synchronization, and (2) what are the precise conditions to switch between the two modes, and what are the expectations about the local state (e.g., incoming message buffers, etc.) when such a switch happens? +Particularly Point 2 is quite hard to get right in a distributed setting. +For instance, a node might locally believe that it is synchronized, while others have actually moved ahead instead. +This creates a lot of algorithmic complexity as well as complications in the analysis of the protocols. We have thus decided to go for another approach: -- Consensus, client and server run in parallel on a node (we don't need to define switching conditions between consensus and synchronization as they are always running together) +- Consensus, client and server run in parallel on a node (we don't need to define switching conditions between consensus and Blocksync as they are always running together) - the consensus logic is the single point where decisions are made (i.e., blocks are committed) -- the synchronization protocol is just an alternative source for certificates and proposals -- the synchronization can be run as add-on, and doesn't need any change to the consensus mechanism/architecture already implemented/specified in Malachite. -- Coupling of synchronization client and server to the consensus logic: - - the server needs read access to the block store in order to retrieve certificates and blocks for committed heights +- Blocksync is just an alternative source for certificates and proposals +- Blocksync can be run as add-on, and doesn't need any change to the consensus mechanism/architecture already implemented/specified in Malachite. +- Coupling of Blocksync client and server to the consensus logic: + - the server needs read access to the block store in order to retrieve the current height, as well as certificates and blocks for committed heights - the client needs write access to incoming buffers (for certificates and blocks) of the consensus logic -## Central aspects of Synchronization +## Central aspects of Blocksync ### Synchronization Messages @@ -60,6 +64,7 @@ A client asks a specific peer either for a certificate or for a block at a given type ReqType = | SyncCertificate | SyncBlock + | SyncBlockStoreEntry type RequestMsg = { client: Address, @@ -75,6 +80,7 @@ A server provides the required information to a client: type Response = | RespBlock(Proposal) | RespCertificate(Set[Vote]) + | RespBlockStoreEntry(BlockStoreEntry) type ResponseMsg = { client: Address, @@ -90,37 +96,45 @@ If a node is behind multiple heights, in principle, we could - employ advanced schemes of incentive-aligned strategies which server to ask for which height In this version we have encoded a very basic mechanism -- the client uses the information from the received `StatusMsg`s to record who can provide what information +- the client uses the information from the received messages `StatusMsg` to record who can provide what information - (A) when a node falls behind - it requests a certificate for the next height from one of the servers that is reported that it has this information - when the server receives the certificate, - - it feeds the certificate into the incoming certificate buffer of the node, and + - it feeds the certificate into the incoming certificate buffer of driver, and - it requests the block from the same server - when the server receives the block, - it feeds the block into the incoming block buffer of the node, - if there are still heights missing, we repeat from (A) -In the section on [Issues](#issues) below we will discuss future improvements. +In the section on [Issues](#issues) below we will discuss future improvements. We have also encoded a mechanism to request complete blockstore entries (blocks together with commits). ## Formalizing the protocol in Quint -We have formalized the synchronization protocol in Quint. +We have formalized Blocksync in Quint. To do so, we abstracted away many details not relevant to the understanding of the protocol. The [specification](../../quint/specs/blocksync/) includes: -- protocol functionality: main complexity in the client, where it maintains statuses, requests data, and feeds received data into consensus -- state machine: We have put the synchronization on-top-of the consensus specification. For analysis, we may abstract consensus in the future. -- invariants (that have been preliminarily tested) and temporal formulas (that are just written but have not been investigated further) +- Protocol functionality: main complexity in the client, where it maintains statuses, requests data, and feeds received data into consensus +- State machine: We have encoded two alternatives + - We have put the Blocksync on-top-of the consensus specification (`bsyncWithConsensus`). This allows us to simulate consensus and Blocksync in one model. + - We have encoded a state machine that abstracts away consensus (`bsyncWithMockedConsensus`) that allows us to analyze Blocksync in isolation. +- Invariants (that have been preliminarily tested) and temporal formulas (that are just written but have not been investigated further) ### Protocol functionality This contains mainly the following functions (and their auxiliary functions): -- `pure def bsyncClient (s: BsyncClient) : ClientResult` +- `pure def bsyncClientLogic (s: BsyncClient, fullEntries: bool) : ClientResult` - this encodes what happens during a step of a client: 1. update peer statuses, 2. if there is no open request, request something 3. otherwise check whether we have a response and act accordingly + 4. `fullEntries` is used to signal whether complete block store entries should be requested (rather than certificate and block separately) + +- `pure def bsyncClientTimeout (s: BsyncClient) : ClientResult` + - this encodes what happens when a request timeouts: + 1. update peer statuses, + 2. send the request to another suitable peer - `pure def syncStatus (server: Server) : StatusMsg` - look into the block store of the node, generate a status message @@ -143,36 +157,45 @@ var requestsBuffer : Address -> Set[RequestMsg] var responsesBuffer : Address -> Set[ResponseMsg] ``` -We added the following actions for a correct process `v`: +We have encoded two different state machines (1) that interacts with the consensus specification and (2) that abstracts consensus. In order to do so, we have separated actions into several modules. +The specification `syncStatemachine.qnt` encodes all actions that are touching only Blocksync state, that is, the don't interact with consensus. +These are the actions for a correct process `v`: - `syncDeliverReq(v)` - `syncDeliverResp(v)` - `syncDeliverStatus(v)` -- `syncStepClient(v)` - `syncStatusStep(v)` - `syncStepServer(v)` -- `syncUpdateServer(v)` +- `syncClientTimeout(v)` + +The deliver actions just take a message out of the corresponding network buffer, and puts it into the incoming buffer of node `v`. The other three actions just call the corresponding functions. + +The following to actions interact with consensus and thus there are two versions for the two mentioned state machines. +- `syncStepClient`: executes the client function and potentially feeds certificates or blocks into consensus +- `syncUpdateServer`: updates the synchronization server state with the latest content of the node's blockchain. + + -The deliver actions just take a message out of the corresponding network buffer, and puts it into the incoming buffer of node `v`. -The next three actions just execute the matching functions discussed above. -The last action updates the synchronization server state with the latest content of the node's blockchain. #### syncStepClient + There are two types of effects this action can have. It can lead to a request message being sent to a server, in which case the message is place in the `requestsBuffer` towards the server. The second effect is that when the client learns a certificate or a proposal, it will be put into an incoming buffer of a node (from which the consensus logic can later take it out and act on it). #### syncStatusStep + A status message is broadcast, that is, the message is put into the `statusBuffer` towards all nodes. #### syncStepServer + I a request is served, the response message is put into the `responsesBuffer` towards the requesting client. ### Invariants and temporal formulas -TODO for the retreat +For details we refer to the [state machine in Quint](https://github.com/informalsystems/malachite/blob/main/specs/quint/specs/blocksync/bsyncStatemachine.qnt), and the [analysis documentation](https://github.com/informalsystems/malachite/blob/main/specs/quint/specs/blocksync/README.md). ## Issues The description refers to a minimal version of the synchronization protocol. -Its roadmap can be found in the issue [#425](https://github.com/informalsystems/malachite/issues/425). \ No newline at end of file +Its roadmap can be found in the issue [#425](https://github.com/informalsystems/malachite/issues/425). diff --git a/specs/quint/specs/blocksync/README.md b/specs/quint/specs/blocksync/README.md new file mode 100644 index 000000000..c481a1881 --- /dev/null +++ b/specs/quint/specs/blocksync/README.md @@ -0,0 +1,22 @@ +# Analysis of the BlockSync Specification + +This document contains a report on analysis of BlockSync. We have two versions of the +state machine, one with consensus abstracted away, and one in which the blocksync +state machine and the consensus state machine are combined. + +## Invariants checked with quint run + +- `validRequestInvariant`: A request should only be sent to a server who has reported, via status message, having data for the requested height. +- `noOldRequestsInv`: A client doesn't have open requests for past heights +- `serverRespondsToRequestingPeersInvariant`: A server only replies to a request received from a client (The client request might have timed out). + +## Witnesses + +- `serverRespondsToRequestingPeersWitness`: This witness should report a scenario where a request timeouts, the client submits a new one, and a late response is received. + +## Temporal properties + +We don't check these properties but record them for documentation purposes. +- `terminationRequest`: Every request will eventually terminate. + + From 28a0c3abff49dd735c20f22b3592c21c4c88d13f Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Fri, 22 Nov 2024 08:30:31 -0500 Subject: [PATCH 2/4] chore(code/blocksync): Include synced certificate in decision if one exists, otherwise derive (#598) * Include synced certificate in decision if one exists, otherwise derive from seen commits. * Add `get_certificate` method on `Driver` --------- Co-authored-by: Romain Ruetschi --- code/crates/consensus/src/handle/decide.rs | 16 +++++++++++++--- code/crates/driver/src/driver.rs | 19 +++++++++++++++---- code/crates/driver/src/mux.rs | 18 ++++++++++-------- 3 files changed, 38 insertions(+), 15 deletions(-) diff --git a/code/crates/consensus/src/handle/decide.rs b/code/crates/consensus/src/handle/decide.rs index e9722b58b..b1a46e7f7 100644 --- a/code/crates/consensus/src/handle/decide.rs +++ b/code/crates/consensus/src/handle/decide.rs @@ -14,8 +14,8 @@ where let proposal_round = proposal.round(); let value = proposal.value(); - // Restore the commits. Note that they will be removed from `state` - let commits = state.restore_precommits(height, proposal_round, value); + // We only decide proposals for the current height + assert_eq!(height, state.driver.height()); // Clean proposals and values state.remove_full_proposals(height); @@ -46,7 +46,17 @@ where } } - let certificate = CommitCertificate::new(height, proposal_round, value.id(), commits); + // Look for an existing certificate + let certificate = state + .driver + .get_certificate(proposal_round, value.id()) + .cloned() + .unwrap_or_else(|| { + // Restore the commits. Note that they will be removed from `state` + let commits = state.restore_precommits(height, proposal_round, value); + // TODO: should we verify we have 2/3rd commits? + CommitCertificate::new(height, proposal_round, value.id(), commits) + }); perform!(co, Effect::Decide { certificate }); diff --git a/code/crates/driver/src/driver.rs b/code/crates/driver/src/driver.rs index 48de6f78c..893ab9112 100644 --- a/code/crates/driver/src/driver.rs +++ b/code/crates/driver/src/driver.rs @@ -4,7 +4,7 @@ use core::fmt; use malachite_common::{ CommitCertificate, Context, Proposal, Round, SignedProposal, SignedVote, Timeout, TimeoutStep, - Validator, ValidatorSet, Validity, Vote, + Validator, ValidatorSet, Validity, ValueId, Vote, }; use malachite_round::input::Input as RoundInput; use malachite_round::output::Output as RoundOutput; @@ -38,6 +38,9 @@ where /// The validator set at the current height validator_set: Ctx::ValidatorSet, + /// The proposer for the current round, None for round nil. + proposer: Option, + /// The proposals to decide on. pub(crate) proposal_keeper: ProposalKeeper, @@ -50,9 +53,6 @@ where /// The state of the round state machine. pub(crate) round_state: RoundState, - /// The proposer for the current round, None for round nil. - proposer: Option, - /// The pending inputs to be processed next, if any. /// The first element of the tuple is the round at which that input has been emitted. pending_inputs: Vec<(Round, RoundInput)>, @@ -176,6 +176,17 @@ where } } + /// Get a commit certificate for the given round and value id. + pub fn get_certificate( + &self, + round: Round, + value_id: ValueId, + ) -> Option<&CommitCertificate> { + self.certificates + .iter() + .find(|c| c.round == round && c.value_id == value_id) + } + /// Process the given input, returning the outputs to be broadcast to the network. pub fn process(&mut self, msg: Input) -> Result>, Error> { let round_output = match self.apply(msg)? { diff --git a/code/crates/driver/src/mux.rs b/code/crates/driver/src/mux.rs index 614441b4a..08d308ddc 100644 --- a/code/crates/driver/src/mux.rs +++ b/code/crates/driver/src/mux.rs @@ -119,11 +119,10 @@ where // We have a valid proposal. Check if there is already a certificate for it. // L49 - if self - .certificates - .iter() - .any(|c| c.value_id == proposal.value().id() && proposal.round() == c.round) - && self.round_state.decision.is_none() + if self.round_state.decision.is_none() + && self + .get_certificate(proposal.round(), proposal.value().id()) + .is_some() { return Some(RoundInput::ProposalAndPrecommitValue(proposal)); } @@ -189,15 +188,18 @@ where // Should only receive proposals for our height. assert_eq!(self.height(), certificate.height); + let certificate_round = certificate.round; + let certificate_value_id = certificate.value_id.clone(); + // Store the certificate - self.certificates.push(certificate.clone()); + self.certificates.push(certificate); if let Some((signed_proposal, validity)) = self .proposal_keeper - .get_proposal_and_validity_for_round(certificate.round) + .get_proposal_and_validity_for_round(certificate_round) { let proposal = &signed_proposal.message; - if proposal.value().id() == certificate.value_id && validity.is_valid() { + if proposal.value().id() == certificate_value_id && validity.is_valid() { return Some(RoundInput::ProposalAndPrecommitValue(proposal.clone())); } } From ced48f9c9a5e66bc632189405cc6331ca2e4eeda Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Fri, 22 Nov 2024 14:57:56 +0100 Subject: [PATCH 3/4] chore(code/test): Move unit tests from `malachite-test` into their respective crates (#597) * Disable `unused_crate_dependencies` lint as it is utterly broken See https://github.com/rust-lang/rust/issues/95513 * Move driver tests into the `driver` crate * Move vote keeper tests into the `vote` crate * Move full proposal keeper tests into the `consensus` crate --------- Co-authored-by: Anca Zamfir --- code/Cargo.lock | 26 ++-- code/Cargo.toml | 10 +- code/crates/common/src/lib.rs | 2 +- code/crates/consensus/Cargo.toml | 3 + .../tests/full_proposal.rs} | 3 +- code/crates/driver/Cargo.toml | 10 +- code/crates/driver/src/lib.rs | 2 +- code/crates/driver/test-utils/Cargo.toml | 18 +++ .../test-utils/src/lib.rs} | 3 +- .../tests/driver.rs => driver/tests/basic.rs} | 0 .../driver_extra.rs => driver/tests/extra.rs} | 2 +- .../gossip-consensus/test/tests/discovery.rs | 2 - code/crates/round/src/lib.rs | 2 +- code/crates/starknet/test/tests/blocksync.rs | 2 - .../test/tests/n2f0_consensus_mode.rs | 2 - .../test/tests/n2f0_pubsub_protocol.rs | 2 - code/crates/starknet/test/tests/n3f0.rs | 2 - code/crates/starknet/test/tests/n3f1.rs | 2 - code/crates/test/Cargo.toml | 11 -- code/crates/test/src/utils/mod.rs | 1 - code/crates/test/tests/round.rs | 144 ------------------ code/crates/vote/Cargo.toml | 3 + code/crates/vote/src/lib.rs | 2 +- .../{test => vote}/tests/round_votes.rs | 0 .../crates/{test => vote}/tests/vote_count.rs | 0 .../{test => vote}/tests/vote_keeper.rs | 0 26 files changed, 60 insertions(+), 194 deletions(-) rename code/crates/{test/tests/full_proposal_keeper.rs => consensus/tests/full_proposal.rs} (99%) create mode 100644 code/crates/driver/test-utils/Cargo.toml rename code/crates/{test/src/utils/driver.rs => driver/test-utils/src/lib.rs} (99%) rename code/crates/{test/tests/driver.rs => driver/tests/basic.rs} (100%) rename code/crates/{test/tests/driver_extra.rs => driver/tests/extra.rs} (99%) delete mode 100644 code/crates/test/tests/round.rs rename code/crates/{test => vote}/tests/round_votes.rs (100%) rename code/crates/{test => vote}/tests/vote_count.rs (100%) rename code/crates/{test => vote}/tests/vote_keeper.rs (100%) diff --git a/code/Cargo.lock b/code/Cargo.lock index 9f54df8ee..db759d810 100644 --- a/code/Cargo.lock +++ b/code/Cargo.lock @@ -2534,6 +2534,7 @@ dependencies = [ "malachite-common", "malachite-driver", "malachite-metrics", + "malachite-test", "multiaddr", "thiserror 2.0.3", "tracing", @@ -2572,11 +2573,24 @@ version = "0.1.0" dependencies = [ "derive-where", "malachite-common", + "malachite-driver-test-utils", "malachite-round", + "malachite-test", "malachite-vote", "thiserror 2.0.3", ] +[[package]] +name = "malachite-driver-test-utils" +version = "0.1.0" +dependencies = [ + "malachite-common", + "malachite-driver", + "malachite-round", + "malachite-test", + "malachite-vote", +] + [[package]] name = "malachite-gossip-consensus" version = "0.1.0" @@ -2755,21 +2769,13 @@ dependencies = [ name = "malachite-test" version = "0.1.0" dependencies = [ - "async-trait", "base64 0.22.1", "bytes", - "bytesize", "ed25519-consensus", "hex", - "malachite-actors", "malachite-common", - "malachite-config", - "malachite-consensus", - "malachite-driver", "malachite-proto", - "malachite-round", "malachite-signing-ed25519", - "malachite-vote", "prost", "prost-build", "prost-types", @@ -2777,9 +2783,6 @@ dependencies = [ "serde", "sha3", "signature", - "tokio", - "tracing", - "tracing-subscriber", ] [[package]] @@ -2808,6 +2811,7 @@ version = "0.1.0" dependencies = [ "derive-where", "malachite-common", + "malachite-test", "thiserror 2.0.3", ] diff --git a/code/Cargo.toml b/code/Cargo.toml index 727753139..623a2cd23 100644 --- a/code/Cargo.toml +++ b/code/Cargo.toml @@ -24,6 +24,7 @@ members = [ # Test "crates/test", "crates/test/mbt", + "crates/driver/test-utils", # Starknet "crates/starknet/*", @@ -50,7 +51,7 @@ inherits = "release" debug = true [workspace.lints.rust] -unused_crate_dependencies = "warn" +# None for now [workspace.dependencies] malachite-actors = { version = "0.1.0", path = "crates/actors" } @@ -67,11 +68,14 @@ malachite-metrics = { version = "0.1.0", path = "crates/metrics" } malachite-node = { version = "0.1.0", path = "crates/node" } malachite-proto = { version = "0.1.0", path = "crates/proto" } malachite-round = { version = "0.1.0", path = "crates/round" } -malachite-test = { version = "0.1.0", path = "crates/test" } -malachite-test-mbt = { version = "0.1.0", path = "crates/test/mbt" } malachite-vote = { version = "0.1.0", path = "crates/vote" } malachite-signing-ed25519 = { version = "0.1.0", path = "crates/signing-ed25519" } +# Test +malachite-test = { version = "0.1.0", path = "crates/test" } +malachite-test-mbt = { version = "0.1.0", path = "crates/test/mbt" } +malachite-driver-test-utils = { version = "0.1.0", path = "crates/driver/test-utils" } + # Starknet malachite-starknet-host = { version = "0.1.0", path = "crates/starknet/host" } malachite-starknet-app = { version = "0.1.0", path = "crates/starknet/app" } diff --git a/code/crates/common/src/lib.rs b/code/crates/common/src/lib.rs index 5d255acb5..6b94909a8 100644 --- a/code/crates/common/src/lib.rs +++ b/code/crates/common/src/lib.rs @@ -2,7 +2,7 @@ #![no_std] #![forbid(unsafe_code)] -#![deny(unused_crate_dependencies, trivial_casts, trivial_numeric_casts)] +#![deny(trivial_casts, trivial_numeric_casts)] #![warn( missing_docs, rustdoc::broken_intra_doc_links, diff --git a/code/crates/consensus/Cargo.toml b/code/crates/consensus/Cargo.toml index 9263b9ca0..fb1253d1d 100644 --- a/code/crates/consensus/Cargo.toml +++ b/code/crates/consensus/Cargo.toml @@ -27,3 +27,6 @@ tracing = { workspace = true } [lints] workspace = true + +[dev-dependencies] +malachite-test = { workspace = true } diff --git a/code/crates/test/tests/full_proposal_keeper.rs b/code/crates/consensus/tests/full_proposal.rs similarity index 99% rename from code/crates/test/tests/full_proposal_keeper.rs rename to code/crates/consensus/tests/full_proposal.rs index 6cf9ea389..34f6b9bdf 100644 --- a/code/crates/test/tests/full_proposal_keeper.rs +++ b/code/crates/consensus/tests/full_proposal.rs @@ -1,6 +1,5 @@ -use malachite_actors::host::ProposedValue; use malachite_common::{Context, Round, SignedProposal, Validity}; -use malachite_consensus::{FullProposal, FullProposalKeeper, Input}; +use malachite_consensus::{FullProposal, FullProposalKeeper, Input, ProposedValue}; use malachite_test::utils::validators::make_validators; use malachite_test::{Address, Proposal, Value}; use malachite_test::{Height, TestContext}; diff --git a/code/crates/driver/Cargo.toml b/code/crates/driver/Cargo.toml index 79caf4c53..d9862d751 100644 --- a/code/crates/driver/Cargo.toml +++ b/code/crates/driver/Cargo.toml @@ -17,9 +17,13 @@ debug = ["std", "malachite-round/debug"] workspace = true [dependencies] -malachite-common = { version = "0.1.0", path = "../common" } -malachite-round = { version = "0.1.0", path = "../round" } -malachite-vote = { version = "0.1.0", path = "../vote" } +malachite-common = { workspace = true } +malachite-round = { workspace = true } +malachite-vote = { workspace = true } derive-where = { workspace = true } thiserror = { workspace = true, default-features = false } + +[dev-dependencies] +malachite-test = { workspace = true } +malachite-driver-test-utils = { workspace = true } diff --git a/code/crates/driver/src/lib.rs b/code/crates/driver/src/lib.rs index fa169655a..f4c21f24d 100644 --- a/code/crates/driver/src/lib.rs +++ b/code/crates/driver/src/lib.rs @@ -1,7 +1,7 @@ //! Driver for the state machine of the Malachite consensus engine #![forbid(unsafe_code)] -#![deny(unused_crate_dependencies, trivial_casts, trivial_numeric_casts)] +#![deny(trivial_casts, trivial_numeric_casts)] #![warn( missing_docs, rustdoc::broken_intra_doc_links, diff --git a/code/crates/driver/test-utils/Cargo.toml b/code/crates/driver/test-utils/Cargo.toml new file mode 100644 index 000000000..fe875cd0e --- /dev/null +++ b/code/crates/driver/test-utils/Cargo.toml @@ -0,0 +1,18 @@ +[package] +name = "malachite-driver-test-utils" +description = "Test utilities for the `malachite-driver`" +publish = false + +version.workspace = true +edition.workspace = true +repository.workspace = true +license.workspace = true +rust-version.workspace = true + + +[dependencies] +malachite-common = { workspace = true } +malachite-round = { workspace = true } +malachite-vote = { workspace = true } +malachite-test = { workspace = true } +malachite-driver = { workspace = true } diff --git a/code/crates/test/src/utils/driver.rs b/code/crates/driver/test-utils/src/lib.rs similarity index 99% rename from code/crates/test/src/utils/driver.rs rename to code/crates/driver/test-utils/src/lib.rs index 83cdd4618..bb8253364 100644 --- a/code/crates/test/src/utils/driver.rs +++ b/code/crates/driver/test-utils/src/lib.rs @@ -1,8 +1,7 @@ use malachite_common::{NilOrVal, Round, SignedProposal, SignedVote, Timeout, Validity}; use malachite_driver::{Input, Output}; use malachite_round::state::{RoundValue, State, Step}; - -use crate::{Address, Height, Proposal, Signature, TestContext, Value, Vote}; +use malachite_test::{Address, Height, Proposal, Signature, TestContext, Value, Vote}; pub fn new_round_input(round: Round, proposer: Address) -> Input { Input::NewRound(Height::new(1), round, proposer) diff --git a/code/crates/test/tests/driver.rs b/code/crates/driver/tests/basic.rs similarity index 100% rename from code/crates/test/tests/driver.rs rename to code/crates/driver/tests/basic.rs diff --git a/code/crates/test/tests/driver_extra.rs b/code/crates/driver/tests/extra.rs similarity index 99% rename from code/crates/test/tests/driver_extra.rs rename to code/crates/driver/tests/extra.rs index f28b82443..355ca5573 100644 --- a/code/crates/test/tests/driver_extra.rs +++ b/code/crates/driver/tests/extra.rs @@ -2,7 +2,7 @@ use malachite_common::{Round, Validity}; use malachite_driver::{Driver, Input, Output}; use malachite_round::state::State; -use malachite_test::utils::driver::*; +use malachite_driver_test_utils::*; use malachite_test::utils::validators::make_validators; use malachite_test::{Height, Proposal, TestContext, ValidatorSet, Value}; diff --git a/code/crates/gossip-consensus/test/tests/discovery.rs b/code/crates/gossip-consensus/test/tests/discovery.rs index 75071a0cb..ffd873d47 100644 --- a/code/crates/gossip-consensus/test/tests/discovery.rs +++ b/code/crates/gossip-consensus/test/tests/discovery.rs @@ -1,5 +1,3 @@ -#![allow(unused_crate_dependencies)] - use std::{time::Duration, vec}; use malachite_discovery_test::{Expected, Test, TestNode}; diff --git a/code/crates/round/src/lib.rs b/code/crates/round/src/lib.rs index e9fa5d67e..6d4352eb8 100644 --- a/code/crates/round/src/lib.rs +++ b/code/crates/round/src/lib.rs @@ -1,7 +1,7 @@ //! Per-round consensus state machine #![forbid(unsafe_code)] -#![deny(unused_crate_dependencies, trivial_casts, trivial_numeric_casts)] +#![deny(trivial_casts, trivial_numeric_casts)] #![warn( missing_docs, rustdoc::broken_intra_doc_links, diff --git a/code/crates/starknet/test/tests/blocksync.rs b/code/crates/starknet/test/tests/blocksync.rs index 914e49483..1b8f542eb 100644 --- a/code/crates/starknet/test/tests/blocksync.rs +++ b/code/crates/starknet/test/tests/blocksync.rs @@ -1,5 +1,3 @@ -#![allow(unused_crate_dependencies)] - use std::time::Duration; use malachite_starknet_test::{Test, TestNode, TestParams}; diff --git a/code/crates/starknet/test/tests/n2f0_consensus_mode.rs b/code/crates/starknet/test/tests/n2f0_consensus_mode.rs index cb678eadf..4fd20626a 100644 --- a/code/crates/starknet/test/tests/n2f0_consensus_mode.rs +++ b/code/crates/starknet/test/tests/n2f0_consensus_mode.rs @@ -1,5 +1,3 @@ -#![allow(unused_crate_dependencies)] - use std::time::Duration; use malachite_config::ValuePayload; diff --git a/code/crates/starknet/test/tests/n2f0_pubsub_protocol.rs b/code/crates/starknet/test/tests/n2f0_pubsub_protocol.rs index 1eeb91d72..14b43e1ab 100644 --- a/code/crates/starknet/test/tests/n2f0_pubsub_protocol.rs +++ b/code/crates/starknet/test/tests/n2f0_pubsub_protocol.rs @@ -1,5 +1,3 @@ -#![allow(unused_crate_dependencies)] - use std::time::Duration; use bytesize::ByteSize; diff --git a/code/crates/starknet/test/tests/n3f0.rs b/code/crates/starknet/test/tests/n3f0.rs index 80855c522..2f2414ec4 100644 --- a/code/crates/starknet/test/tests/n3f0.rs +++ b/code/crates/starknet/test/tests/n3f0.rs @@ -1,5 +1,3 @@ -#![allow(unused_crate_dependencies)] - use std::time::Duration; use malachite_starknet_test::{Test, TestNode}; diff --git a/code/crates/starknet/test/tests/n3f1.rs b/code/crates/starknet/test/tests/n3f1.rs index 7b3159a07..1c5b3d23a 100644 --- a/code/crates/starknet/test/tests/n3f1.rs +++ b/code/crates/starknet/test/tests/n3f1.rs @@ -1,5 +1,3 @@ -#![allow(unused_crate_dependencies)] - use std::time::Duration; use malachite_starknet_test::{Test, TestNode}; diff --git a/code/crates/test/Cargo.toml b/code/crates/test/Cargo.toml index ab862df72..78f979fd9 100644 --- a/code/crates/test/Cargo.toml +++ b/code/crates/test/Cargo.toml @@ -10,19 +10,11 @@ license.workspace = true rust-version.workspace = true [dependencies] -malachite-actors = { workspace = true } malachite-common = { workspace = true } -malachite-consensus = { workspace = true } -malachite-driver = { workspace = true } -malachite-config = { workspace = true } -malachite-round = { workspace = true } -malachite-vote = { workspace = true } malachite-proto = { workspace = true } malachite-signing-ed25519 = { workspace = true, features = ["rand", "serde"] } -async-trait = { workspace = true } base64 = { workspace = true } -bytesize = { workspace = true } bytes = { workspace = true } ed25519-consensus = { workspace = true } hex = { workspace = true } @@ -32,9 +24,6 @@ rand = { workspace = true } serde = { workspace = true, features = ["derive"] } sha3 = { workspace = true } signature = { workspace = true } -tokio = { workspace = true } -tracing = { workspace = true } -tracing-subscriber = { workspace = true, features = ["fmt", "env-filter"] } [build-dependencies] prost-build = { workspace = true } diff --git a/code/crates/test/src/utils/mod.rs b/code/crates/test/src/utils/mod.rs index 71851b6ff..f3826ce00 100644 --- a/code/crates/test/src/utils/mod.rs +++ b/code/crates/test/src/utils/mod.rs @@ -1,2 +1 @@ -pub mod driver; pub mod validators; diff --git a/code/crates/test/tests/round.rs b/code/crates/test/tests/round.rs deleted file mode 100644 index 4b8a5b297..000000000 --- a/code/crates/test/tests/round.rs +++ /dev/null @@ -1,144 +0,0 @@ -use malachite_test::{Address, Height, Proposal, TestContext, Value}; - -use malachite_common::{NilOrVal, Round, Timeout, TimeoutStep}; -use malachite_round::input::Input; -use malachite_round::output::Output; -use malachite_round::state::{State, Step}; -use malachite_round::state_machine::{apply, Info}; - -const ADDRESS: Address = Address::new([42; 20]); -const OTHER_ADDRESS: Address = Address::new([21; 20]); - -#[test] -fn test_propose() { - let value = Value::new(42); - let height = Height::new(10); - let round = Round::new(0); - - let mut state: State = State { - height, - round, - ..Default::default() - }; - - // We are the proposer - let data = Info::new(round, &ADDRESS, &ADDRESS); - - let transition = apply(state.clone(), &data, Input::NewRound(round)); - - state.step = Step::Propose; - assert_eq!(transition.next_state, state); - assert_eq!( - transition.output.unwrap(), - Output::get_value_and_schedule_timeout(height, round, TimeoutStep::Propose) - ); - - let transition = apply(transition.next_state, &data, Input::ProposeValue(value)); - - state.step = Step::Propose; - assert_eq!(transition.next_state, state); - assert_eq!( - transition.output.unwrap(), - Output::proposal( - Height::new(10), - Round::new(0), - Value::new(42), - Round::Nil, - ADDRESS - ) - ); -} - -#[test] -fn test_prevote() { - let value = Value::new(42); - let height = Height::new(1); - let round = Round::new(1); - - let state: State = State { - height, - round, - ..Default::default() - }; - - // We are not the proposer - let data = Info::new(round, &ADDRESS, &OTHER_ADDRESS); - - let transition = apply(state, &data, Input::NewRound(round)); - - assert_eq!(transition.next_state.step, Step::Propose); - assert_eq!( - transition.output.unwrap(), - Output::ScheduleTimeout(Timeout { - round: Round::new(1), - step: TimeoutStep::Propose - }) - ); - - let state = transition.next_state; - - let transition = apply( - state, - &data, - Input::Proposal(Proposal::new( - Height::new(1), - Round::new(1), - value, - Round::Nil, - OTHER_ADDRESS, - )), - ); - - assert_eq!(transition.next_state.step, Step::Prevote); - assert_eq!( - transition.output.unwrap(), - Output::prevote( - Height::new(1), - Round::new(1), - NilOrVal::Val(value.id()), - ADDRESS - ) - ); -} - -#[test] -fn test_input_message_while_commit_step() { - let value = Value::new(42); - let height = Height::new(1); - let round = Round::new(1); - - let state: State = State { - height, - round, - ..Default::default() - }; - - let proposal = Proposal::new( - Height::new(1), - Round::new(1), - value, - Round::Nil, - OTHER_ADDRESS, - ); - - let data = Info::new(round, &ADDRESS, &OTHER_ADDRESS); - - let mut transition = apply(state, &data, Input::NewRound(round)); - let mut state = transition.next_state; - - // Go to Commit step via L49 - transition = apply( - state, - &data, - Input::ProposalAndPrecommitValue(proposal.clone()), - ); - state = transition.next_state; - assert_eq!(state.step, Step::Commit); - - // Send a proposal message while in Commit step, transition should be invalid - transition = apply(state, &data, Input::Proposal(proposal)); - state = transition.next_state; - - assert_eq!(state.step, Step::Commit); - assert!(!transition.valid); -} diff --git a/code/crates/vote/Cargo.toml b/code/crates/vote/Cargo.toml index 5bebf5517..965990705 100644 --- a/code/crates/vote/Cargo.toml +++ b/code/crates/vote/Cargo.toml @@ -14,3 +14,6 @@ malachite-common = { version = "0.1.0", path = "../common" } derive-where = { workspace = true } thiserror = { workspace = true, default-features = false } + +[dev-dependencies] +malachite-test = { workspace = true } diff --git a/code/crates/vote/src/lib.rs b/code/crates/vote/src/lib.rs index 63c5502d9..9116303fa 100644 --- a/code/crates/vote/src/lib.rs +++ b/code/crates/vote/src/lib.rs @@ -2,7 +2,7 @@ #![no_std] #![forbid(unsafe_code)] -#![deny(unused_crate_dependencies, trivial_casts, trivial_numeric_casts)] +#![deny(trivial_casts, trivial_numeric_casts)] #![warn( missing_docs, rustdoc::broken_intra_doc_links, diff --git a/code/crates/test/tests/round_votes.rs b/code/crates/vote/tests/round_votes.rs similarity index 100% rename from code/crates/test/tests/round_votes.rs rename to code/crates/vote/tests/round_votes.rs diff --git a/code/crates/test/tests/vote_count.rs b/code/crates/vote/tests/vote_count.rs similarity index 100% rename from code/crates/test/tests/vote_count.rs rename to code/crates/vote/tests/vote_count.rs diff --git a/code/crates/test/tests/vote_keeper.rs b/code/crates/vote/tests/vote_keeper.rs similarity index 100% rename from code/crates/test/tests/vote_keeper.rs rename to code/crates/vote/tests/vote_keeper.rs From a37d307bd0ba54067f044168e5eaaa33ef0f652e Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Fri, 22 Nov 2024 15:00:44 +0100 Subject: [PATCH 4/4] chore(code/consensus): Remove `SyncedBlock` effect and rename some inputs (#593) * chore(code/consensus): Remove `SyncedBlock` effect, rename `ReceivedSyncedBlock` input to `CommitCertificate` * Rename `ProposeValue` input to `Propose` to avoid ambiguity with `ProposedValue` input --------- Co-authored-by: Anca Zamfir --- code/Cargo.lock | 1 - code/crates/actors/src/consensus.rs | 54 ++++++++----------- code/crates/actors/src/host.rs | 2 +- code/crates/consensus/Cargo.toml | 1 - code/crates/consensus/src/effect.rs | 10 ---- code/crates/consensus/src/handle.rs | 34 ++++-------- .../handle/{propose_value.rs => propose.rs} | 21 ++++---- ...ed_proposed_value.rs => proposed_value.rs} | 4 +- .../src/handle/{synced_block.rs => sync.rs} | 15 +----- code/crates/consensus/src/input.rs | 27 +++------- code/crates/consensus/src/state.rs | 4 ++ code/crates/consensus/src/types.rs | 12 ++++- code/crates/consensus/tests/full_proposal.rs | 4 +- code/crates/starknet/host/src/actor.rs | 2 +- 14 files changed, 74 insertions(+), 117 deletions(-) rename code/crates/consensus/src/handle/{propose_value.rs => propose.rs} (75%) rename code/crates/consensus/src/handle/{received_proposed_value.rs => proposed_value.rs} (94%) rename code/crates/consensus/src/handle/{synced_block.rs => sync.rs} (76%) diff --git a/code/Cargo.lock b/code/Cargo.lock index db759d810..ae23f1a11 100644 --- a/code/Cargo.lock +++ b/code/Cargo.lock @@ -2527,7 +2527,6 @@ name = "malachite-consensus" version = "0.1.0" dependencies = [ "async-recursion", - "bytes", "derive-where", "genawaiter", "libp2p-identity", diff --git a/code/crates/actors/src/consensus.rs b/code/crates/actors/src/consensus.rs index 957ea3c31..b533dd62e 100644 --- a/code/crates/actors/src/consensus.rs +++ b/code/crates/actors/src/consensus.rs @@ -14,7 +14,7 @@ use malachite_common::{ CommitCertificate, Context, Round, SignedExtension, Timeout, TimeoutStep, ValidatorSet, }; use malachite_config::TimeoutConfig; -use malachite_consensus::{Effect, Resume}; +use malachite_consensus::{Effect, Resume, ValueToPropose}; use malachite_metrics::Metrics; use crate::block_sync::BlockSyncRef; @@ -233,7 +233,13 @@ where .process_input( &myself, state, - ConsensusInput::ProposeValue(height, round, Round::Nil, value, extension), + ConsensusInput::Propose(ValueToPropose { + height, + round, + valid_round: Round::Nil, + value, + extension, + }), ) .await; @@ -296,14 +302,24 @@ where return Ok(()); }; + self.host.call_and_forward( + |reply_to| HostMsg::ProcessSyncedBlock { + height: block.certificate.height, + round: block.certificate.round, + validator_address: state.consensus.address().clone(), + block_bytes: block.block_bytes.clone(), + reply_to, + }, + &myself, + |proposed| Msg::::ReceivedProposedValue(proposed), + None, + )?; + if let Err(e) = self .process_input( &myself, state, - ConsensusInput::ReceivedSyncedBlock( - block.block_bytes, - block.certificate, - ), + ConsensusInput::CommitCertificate(block.certificate), ) .await { @@ -404,7 +420,7 @@ where Msg::ReceivedProposedValue(value) => { let result = self - .process_input(&myself, state, ConsensusInput::ReceivedProposedValue(value)) + .process_input(&myself, state, ConsensusInput::ProposedValue(value)) .await; if let Err(e) = result { @@ -601,30 +617,6 @@ where Ok(Resume::Continue) } - - Effect::SyncedBlock { - height, - round, - validator_address, - block_bytes, - } => { - debug!(%height, "Consensus received synced block, sending to host"); - - self.host.call_and_forward( - |reply_to| HostMsg::ProcessSyncedBlockBytes { - height, - round, - validator_address, - block_bytes, - reply_to, - }, - myself, - |proposed| Msg::::ReceivedProposedValue(proposed), - None, - )?; - - Ok(Resume::Continue) - } } } } diff --git a/code/crates/actors/src/host.rs b/code/crates/actors/src/host.rs index a98338234..427d2a53d 100644 --- a/code/crates/actors/src/host.rs +++ b/code/crates/actors/src/host.rs @@ -104,7 +104,7 @@ pub enum HostMsg { }, // Synced block - ProcessSyncedBlockBytes { + ProcessSyncedBlock { height: Ctx::Height, round: Round, validator_address: Ctx::Address, diff --git a/code/crates/consensus/Cargo.toml b/code/crates/consensus/Cargo.toml index fb1253d1d..56668358d 100644 --- a/code/crates/consensus/Cargo.toml +++ b/code/crates/consensus/Cargo.toml @@ -17,7 +17,6 @@ malachite-driver.workspace = true malachite-metrics.workspace = true async-recursion = { workspace = true } -bytes = { workspace = true, features = ["serde"] } genawaiter = { workspace = true } derive-where = { workspace = true } libp2p-identity = { workspace = true, features = ["peerid"] } diff --git a/code/crates/consensus/src/effect.rs b/code/crates/consensus/src/effect.rs index f924d71cc..8155c6f3d 100644 --- a/code/crates/consensus/src/effect.rs +++ b/code/crates/consensus/src/effect.rs @@ -1,4 +1,3 @@ -use bytes::Bytes; use derive_where::derive_where; use malachite_common::*; @@ -62,15 +61,6 @@ where /// Consensus has decided on a value /// Resume with: [`Resume::Continue`] Decide { certificate: CommitCertificate }, - - /// Consensus has received a synced decided block - /// Resume with: [`Resume::Continue`] - SyncedBlock { - height: Ctx::Height, - round: Round, - validator_address: Ctx::Address, - block_bytes: Bytes, - }, } /// A value with which the consensus process can be resumed after yielding an [`Effect`]. diff --git a/code/crates/consensus/src/handle.rs b/code/crates/consensus/src/handle.rs index ac34f7a69..e54162596 100644 --- a/code/crates/consensus/src/handle.rs +++ b/code/crates/consensus/src/handle.rs @@ -3,20 +3,20 @@ use crate::prelude::*; mod decide; mod driver; mod proposal; -mod propose_value; -mod received_proposed_value; +mod propose; +mod proposed_value; mod signature; mod start_height; -mod synced_block; +mod sync; mod timeout; mod validator_set; mod vote; use proposal::on_proposal; -use propose_value::propose_value; -use received_proposed_value::on_received_proposed_value; +use propose::on_propose; +use proposed_value::on_proposed_value; use start_height::reset_and_start_height; -use synced_block::on_received_synced_block; +use sync::on_commit_certificate; use timeout::on_timeout_elapsed; use vote::on_vote; @@ -48,25 +48,11 @@ where } Input::Vote(vote) => on_vote(co, state, metrics, vote).await, Input::Proposal(proposal) => on_proposal(co, state, metrics, proposal).await, - Input::ProposeValue(height, round, valid_round, value, extension) => { - propose_value( - co, - state, - metrics, - height, - round, - valid_round, - value, - extension, - ) - .await - } + Input::Propose(value) => on_propose(co, state, metrics, value).await, Input::TimeoutElapsed(timeout) => on_timeout_elapsed(co, state, metrics, timeout).await, - Input::ReceivedProposedValue(value) => { - on_received_proposed_value(co, state, metrics, value).await - } - Input::ReceivedSyncedBlock(block_bytes, commits) => { - on_received_synced_block(co, state, metrics, block_bytes, commits).await + Input::ProposedValue(value) => on_proposed_value(co, state, metrics, value).await, + Input::CommitCertificate(certificate) => { + on_commit_certificate(co, state, metrics, certificate).await } } } diff --git a/code/crates/consensus/src/handle/propose_value.rs b/code/crates/consensus/src/handle/propose.rs similarity index 75% rename from code/crates/consensus/src/handle/propose_value.rs rename to code/crates/consensus/src/handle/propose.rs index 12e59ab11..bc1bb87a7 100644 --- a/code/crates/consensus/src/handle/propose_value.rs +++ b/code/crates/consensus/src/handle/propose.rs @@ -1,22 +1,25 @@ use crate::prelude::*; use crate::handle::driver::apply_driver_input; -use crate::types::ProposedValue; +use crate::types::{ProposedValue, ValueToPropose}; -#[allow(clippy::too_many_arguments)] -pub async fn propose_value( +pub async fn on_propose( co: &Co, state: &mut State, metrics: &Metrics, - height: Ctx::Height, - round: Round, - valid_round: Round, - value: Ctx::Value, - extension: Option>, + value: ValueToPropose, ) -> Result<(), Error> where Ctx: Context, { + let ValueToPropose { + height, + round, + valid_round, + value, + extension, + } = value; + if state.driver.height() != height { warn!( "Ignoring proposal for height {height}, current height: {}", @@ -41,7 +44,7 @@ where height, round, valid_round, - validator_address: state.driver.address().clone(), + validator_address: state.address().clone(), value: value.clone(), validity: Validity::Valid, extension, diff --git a/code/crates/consensus/src/handle/received_proposed_value.rs b/code/crates/consensus/src/handle/proposed_value.rs similarity index 94% rename from code/crates/consensus/src/handle/received_proposed_value.rs rename to code/crates/consensus/src/handle/proposed_value.rs index ec1694464..6c8f48957 100644 --- a/code/crates/consensus/src/handle/received_proposed_value.rs +++ b/code/crates/consensus/src/handle/proposed_value.rs @@ -12,7 +12,7 @@ use crate::types::ProposedValue; id = %proposed_value.value.id() ) )] -pub async fn on_received_proposed_value( +pub async fn on_proposed_value( co: &Co, state: &mut State, metrics: &Metrics, @@ -31,7 +31,7 @@ where debug!("Received value for next height, queuing for later"); state .input_queue - .push_back(Input::ReceivedProposedValue(proposed_value)); + .push_back(Input::ProposedValue(proposed_value)); } return Ok(()); } diff --git a/code/crates/consensus/src/handle/synced_block.rs b/code/crates/consensus/src/handle/sync.rs similarity index 76% rename from code/crates/consensus/src/handle/synced_block.rs rename to code/crates/consensus/src/handle/sync.rs index f84b5d89b..91c56961d 100644 --- a/code/crates/consensus/src/handle/synced_block.rs +++ b/code/crates/consensus/src/handle/sync.rs @@ -1,16 +1,13 @@ use std::borrow::Borrow; -use bytes::Bytes; - use crate::handle::driver::apply_driver_input; use crate::handle::validator_set::get_validator_set; use crate::prelude::*; -pub async fn on_received_synced_block( +pub async fn on_commit_certificate( co: &Co, state: &mut State, metrics: &Metrics, - block_bytes: Bytes, certificate: CommitCertificate, ) -> Result<(), Error> where @@ -22,16 +19,6 @@ where "Processing certificate" ); - perform!( - co, - Effect::SyncedBlock { - height: certificate.height, - round: certificate.round, - validator_address: state.driver.address().clone(), - block_bytes, - } - ); - let Some(validator_set) = get_validator_set(co, state, certificate.height).await? else { return Err(Error::ValidatorSetNotFound(certificate.height)); }; diff --git a/code/crates/consensus/src/input.rs b/code/crates/consensus/src/input.rs index 5952cd4ef..2d1b04f94 100644 --- a/code/crates/consensus/src/input.rs +++ b/code/crates/consensus/src/input.rs @@ -1,11 +1,9 @@ -use bytes::Bytes; use derive_where::derive_where; -use malachite_common::{ - CommitCertificate, Context, Round, SignedExtension, SignedProposal, SignedVote, Timeout, -}; +use malachite_common::{CommitCertificate, Context, SignedProposal, SignedVote, Timeout}; use crate::types::ProposedValue; +use crate::ValueToPropose; /// Inputs to be handled by the consensus process. #[derive_where(Clone, Debug, PartialEq, Eq)] @@ -23,25 +21,14 @@ where Proposal(SignedProposal), /// Propose a value - ProposeValue( - /// Height - Ctx::Height, - /// Round - Round, - /// Valid round - Round, - /// Value - Ctx::Value, - /// Signed vote extension - Option>, - ), + Propose(ValueToPropose), /// A timeout has elapsed TimeoutElapsed(Timeout), - /// The value corresponding to a proposal has been received - ReceivedProposedValue(ProposedValue), + /// Received the full proposed value corresponding to a proposal + ProposedValue(ProposedValue), - /// A block received via BlockSync - ReceivedSyncedBlock(Bytes, CommitCertificate), + /// Received a commit certificate from BlockSync + CommitCertificate(CommitCertificate), } diff --git a/code/crates/consensus/src/state.rs b/code/crates/consensus/src/state.rs index 8722cba90..a33367419 100644 --- a/code/crates/consensus/src/state.rs +++ b/code/crates/consensus/src/state.rs @@ -61,6 +61,10 @@ where } } + pub fn address(&self) -> &Ctx::Address { + self.driver.address() + } + pub fn get_proposer(&self, height: Ctx::Height, round: Round) -> &Ctx::Address { self.ctx .select_proposer(self.driver.validator_set(), height, round) diff --git a/code/crates/consensus/src/types.rs b/code/crates/consensus/src/types.rs index 8bf2b7c63..4c8bb0573 100644 --- a/code/crates/consensus/src/types.rs +++ b/code/crates/consensus/src/types.rs @@ -30,8 +30,18 @@ pub enum ConsensusMsg { Proposal(Ctx::Proposal), } +/// A value to propose by the current node. +/// Used only when the node is the proposer. +#[derive_where(Clone, Debug, PartialEq, Eq)] +pub struct ValueToPropose { + pub height: Ctx::Height, + pub round: Round, + pub valid_round: Round, + pub value: Ctx::Value, + pub extension: Option>, +} + /// A value proposed by a validator -/// Called at non-proposer only. #[derive_where(Clone, Debug, PartialEq, Eq)] pub struct ProposedValue { pub height: Ctx::Height, diff --git a/code/crates/consensus/tests/full_proposal.rs b/code/crates/consensus/tests/full_proposal.rs index 34f6b9bdf..029e874ee 100644 --- a/code/crates/consensus/tests/full_proposal.rs +++ b/code/crates/consensus/tests/full_proposal.rs @@ -66,7 +66,7 @@ fn val_msg( value: u64, validity: Validity, ) -> Input { - Input::ReceivedProposedValue(ProposedValue { + Input::ProposedValue(ProposedValue { height: Height::new(1), round: Round::new(round), valid_round: Round::Nil, @@ -275,7 +275,7 @@ fn full_proposal_keeper_tests() { for m in s.input { match m { Input::Proposal(p) => keeper.store_proposal(p), - Input::ReceivedProposedValue(v) => keeper.store_value(&v), + Input::ProposedValue(v) => keeper.store_value(&v), _ => continue, } } diff --git a/code/crates/starknet/host/src/actor.rs b/code/crates/starknet/host/src/actor.rs index 65140a17a..ad72f6ce6 100644 --- a/code/crates/starknet/host/src/actor.rs +++ b/code/crates/starknet/host/src/actor.rs @@ -625,7 +625,7 @@ impl Actor for StarknetHost { Ok(()) } - HostMsg::ProcessSyncedBlockBytes { + HostMsg::ProcessSyncedBlock { height, round, validator_address,