fix: don't ignore valid deposits when another one fails #4165
Conversation
PRO-944 [BUG] Deposits can be ignored
How to reproduceCan the bug be reproduced? How? Ideally, provide a bouncer test or unit test that demonstrated the problem. If we submit a deposit to a non-existent (or expired) channel, then the extrinsic aborts and fails, and any subsequent deposits in the same block are ignored. SymptomsHow does the bug manifest? If it's not obvious, what behaviour did we expect? Swaps can fail randomly due to other users submitting swaps too late. EvidencePlease add any additional supporting metadata such as log output, stack traces, screenshots. Failed btc deposit witness: https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Fperseverance.chainflip.xyz#/explorer/query/784986 FAOTag some relevant people who can look into this. |
Codecov Report
@@ Coverage Diff @@
## main #4165 +/- ##
=====================================
Coverage 72% 72%
=====================================
Files 379 379
Lines 61581 61594 +13
Branches 61581 61594 +13
=====================================
+ Hits 44161 44176 +15
- Misses 15122 15123 +1
+ Partials 2298 2295 -3
... and 4 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Should be ready for re-review |
| assert_eq!(emitted.len(), 2); | ||
| assert!(emitted | ||
| .iter() | ||
| .filter(|e| matches!( |
There was a problem hiding this comment.
can use find instead of filter().next()
There was a problem hiding this comment.
which I see you did in the last commit haha 👍 LGTM
There was a problem hiding this comment.
... And then clippy told me to change it again 😿
Pull Request
Closes: PRO-944
Checklist
Please conduct a thorough self-review before opening the PR.
Summary
We can witness multiple deposits in a single extrinsic. Previously, if any of these deposits were invalid (because the channel had expired already), then all the witnessed deposits would fail. This change will cause every deposit witnessing extrinsic to succeed, but will emit a DepositIgnored event in case one of the deposits was invalid.