Implementation of local signer state machine #5933
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5933 +/- ##
============================================
+ Coverage 68.14% 81.13% +12.99%
============================================
Files 521 522 +1
Lines 382814 383513 +699
Branches 323 323
============================================
+ Hits 260884 311182 +50298
+ Misses 121922 72323 -49599
Partials 8 8
... and 307 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
assertion should not include phantom txs. timing changes around the signer status checks made this more likely.
…us check response
kantai
changed the title
kantai
changed the title
| let consensus_hash = burn_block_event | ||
| .consensus_hash | ||
| .get(2..) | ||
| .ok_or_else(|| EventError::Deserialize("Hex string should be 0x prefixed".into())) | ||
| .and_then(|hex| { | ||
| ConsensusHash::from_hex(hex) | ||
| .map_err(|e| EventError::Deserialize(format!("Invalid hex string: {e}"))) | ||
| })?; | ||
|
|
There was a problem hiding this comment.
This code pattern is repeated for different data (consensus_hash, burn_header_hash, signer_sighash, block_id) . Also the check for "0x" doesn't seem really strong.
What about put this kind of conversion in dedicated function, or eventually even creating a Type::from_hex_0x(...) for each data type? So it could become also testable with a simple unit test and eventually reusable for the future.
|
|
||
| /// The local signer state machine | ||
| #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] | ||
| pub enum LocalStateMachine { |
There was a problem hiding this comment.
Interesting choice to represent the state machine as an enum.
I would appreciate if you could share whether you evaluated the possibility to make the state machine like a struct (currently with a field that represent the current state, but with the possibility have more fields if needed) and, if so, the reasoning behind the choice of this solution.
| }; | ||
|
|
||
| let current_miner = match state_machine.current_miner { | ||
| MinerState::ActiveMiner { |
There was a problem hiding this comment.
Just a nit, but might be cleaner to implement a From for StateMachineUpdateMinerState
This PR contains an initial implementation (and the beginning of testing) for the local state machine work (#5915)