test: bitcoin block processor #5603
Conversation
marcellorigotti
requested review from
MxmUrw
and removed request for
dandanlen and
kylezs
| @@ -141,7 +140,7 @@ fn generate_new_reorg_id<'a, N: BlockZero + SaturatingStep + Ord + 'static>( | |||
| } | |||
|
|
|||
| #[derive(Debug, Clone, PartialEq, Eq, Encode, Decode, TypeInfo, Deserialize, Serialize)] | |||
| pub enum ChainProgressInner<ChainBlockNumber> { | |||
| pub enum ChainProgressInner<ChainBlockNumber: Step> { | |||
There was a problem hiding this comment.
We probably should use SaturatingStep here.
| type CleanOld: for<'a> Hook< | ||
| ( | ||
| &'a mut BTreeMap<Self::ChainBlockNumber, (Self::BlockData, Self::ChainBlockNumber)>, | ||
| &'a mut BTreeMap<Self::ChainBlockNumber, Vec<Self::Event>>, | ||
| Self::ChainBlockNumber, | ||
| ), | ||
| (), | ||
| > + Default | ||
| + Serde | ||
| + Debug | ||
| + Clone | ||
| + Eq; |
There was a problem hiding this comment.
Is the implementation of CleanOld chain-specific? It seems to me that it could be simply a method in the BlockProcessor struct?
| pub trait InnerEquality { | ||
| fn inner_eq(&self, other: &Self) -> bool; | ||
| } |
There was a problem hiding this comment.
I've been thinking about this, and maybe there is a way to do this without this trait. If we split the event into two parts of "blockdata" and "deposit data", then we could compare the deposit data independently of the blockheight/hash to check which events we already emitted. The Execute hook would then take two parameters, i.e.
Hook<(Self::ChainBlockNumber, Self::Event), ()>There was a problem hiding this comment.
We probably want to rename this one back to state_machine.
| type DedupEvents = DedupEventsHook; | ||
|
|
||
| fn get_safety_margin() -> Self::ChainBlockNumber { |
There was a problem hiding this comment.
I think this should be a hook, then we can give it a random value in prop tests. Which would allow us to check more possible edge cases.
- generalize it, general logic the same for every chain - keep hook implementation dependant on the chain for custom behaviour
…counter for testing
TODO: write all the tests
marcellorigotti
force-pushed
the
feature/pro-1985
branch
from
073668c to
5e67410
Compare
Pull Request
Closes: PRO-1985
Checklist
Please conduct a thorough self-review before opening the PR.
Summary
Please include a succinct description of the purpose and content of the PR. What problem does it solve, and how? Link issues, discussions, other PRs, and anything else that will help the reviewer.