feat: Alphanet #651
feat: Alphanet #651
Conversation
nopestack
changed the title
nopestack
added
bug
# Conflicts: # Cargo.lock # Cargo.toml # crates/node/src/consensus/consensus_event_handler.rs # crates/node/src/runtime/node_runtime_handler.rs # crates/node/src/state_manager/dag.rs # crates/node/src/test_utils/mod.rs # examples/multichain.rs
eureka-cpu
changed the title
eureka-cpu
added
big
| let validator_count = (n.clone() as f64 * 0.8).ceil() as usize; | ||
| let farmer_count = (validator_count.clone() as f64 * 0.7).ceil() as usize; | ||
| let harvester_count = validator_count.clone() - farmer_count; |
There was a problem hiding this comment.
u16 and usize both implement Copy so you can remove the .clone()s here (pretty sure)
# Conflicts: # Cargo.toml # crates/node/src/consensus/consensus_module.rs # crates/node/src/state_manager/manager.rs # crates/node/src/state_manager/mod.rs # crates/storage/vrrbdb/src/vrrbdb.rs # examples/multichain.rs
| genesis_block: GenesisBlock, | ||
| ) -> Result<Certificate> { | ||
| let sig = todo!(); | ||
| self.certify_genesis_block(genesis_block, self.config.id.clone(), sig) | ||
| } |
There was a problem hiding this comment.
Just for posterity, anyone else reviewing, etc, this method is just moved. The inner method that is called is deleted and the logic within the deleted method is moved into the new method with this name.
There was a problem hiding this comment.
For some reason the diff I selected isn't showing up. The method is called handle_convergence_block_certificate_received and the logic is moved to handle_convergence_block_certificate_created
| "Could not append convergence block to DAG: {err:?}" | ||
| )) | ||
| })?; | ||
| pub fn handle_block_received(&mut self, block: Block) -> Result<Event> { |
There was a problem hiding this comment.
This is a pretty big change to the return type. I've tracked it down to where the Event is created, and it returns only the hash of the block (or the whole block), where it used to return an ApplyBlockResult which gave us a bit more information. This sort of feels like a regression to me, especially in the unit test cases where we used to be able to get the hashes of both state and transactions, now we only get a single hash to assert against.
Maybe my opinion here is invalid, @ASmithOWL @mitch-vrrb would you mind weighing in? Can we have the event send over the ApplyBlockResult instead of just the hash, that way the logic is maintained, or is it unreasonable to do so?
# Conflicts: # Cargo.lock # Cargo.toml # crates/node/src/test_utils/node_network.rs # examples/multichain.rs
# Conflicts: # Cargo.lock # crates/events/src/event.rs # crates/miner/src/test_helpers.rs # crates/node/src/state_manager/handler.rs # crates/node/src/test_utils/mod.rs # crates/node/tests/genesis_rewards.rs
Refactor error handling in node_runtime_handler.rs to avoid exiting early causing the network to crash.
Ensure the network protocol is fully functional.
Closes #640
Closes #584