feat: Add prewitness deposit events #4745
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4745 +/- ##
======================================
- Coverage 72% 72% -0%
======================================
Files 421 409 -12
Lines 70683 70010 -673
Branches 70683 70010 -673
======================================
- Hits 51220 50654 -566
+ Misses 17037 16925 -112
- Partials 2426 2431 +5 ☔ View full report in Codecov by Sentry. |
| @@ -626,14 +643,38 @@ pub mod pallet { | |||
| DepositBoosted { | |||
| deposit_address: TargetChainAccount<T, I>, | |||
| asset: TargetChainAsset<T, I>, | |||
| amount: TargetChainAmount<T, I>, | |||
| amounts: BTreeMap<BoostPoolTier, TargetChainAmount<T, I>>, | |||
There was a problem hiding this comment.
Minor nit: I think we should consider changing this into a Vec<(BoostPoolTier, TargetChainAmount)>, it's a bit more json-friendly (although this is SCALE so maybe it doesn't matter).
But also - doesn't the boost_id already uniquely identify the amounts?
There was a problem hiding this comment.
BoostPoolTier will be unique, that's why i explicitly usedBTreeMap. I'm pretty sure SCALE will make it output the same as a vec. I prefer to leave it as is. Can change it later if we find its not suitable.
There was a problem hiding this comment.
Yeah that's fine. I was just imagining a future where we have more Tiers, or have more complex tiers (for example a tuple of (Price, MinConfirmations)). Just something to keep to in mind. We can change it later if needed.
| deposit_details: <T::TargetChain as Chain>::DepositDetails, | ||
| boost_id: BoostId, |
There was a problem hiding this comment.
Also I just realised that boost_id is just the prewitnessed_deposit_id. It's a bit strange that we use these interchangeably despite them having different type aliases... Why do we distinguish these?
| boost_fee: TargetChainAmount<T, I>, | ||
| action: DepositAction<T::AccountId>, | ||
| }, | ||
| BoostFundsAdded { | ||
| account_id: T::AccountId, |
There was a problem hiding this comment.
We sometimes use booster_id and sometimes account_id. Why not use booster_id everywhere we talk about boosting?
j4m1ef0rd
force-pushed
the
feat/add-prewitness-deposit-events
branch
from
4ea25da to
8a8e1e1
Compare
Pull Request
Closes: PRO-1295, PRO-1296, PRO-1294, PRO-1293
Checklist
Please conduct a thorough self-review before opening the PR.
Summary
Still not very confident on the details/types in these events, but it's hopefully a good start.
DepositBoostedto have the amounts of each pool used.TODO: Test on localnet.