-
Notifications
You must be signed in to change notification settings - Fork 82
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Separate the validation from the execution process for send/mint/burn_coins
operations
#512
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #512 +/- ##
==========================================
- Coverage 71.94% 71.80% -0.15%
==========================================
Files 126 126
Lines 15771 15820 +49
==========================================
+ Hits 11347 11359 +12
- Misses 4424 4461 +37
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great job on this PR, it was a tricky one!
…n_coins` operations (#512) * Add sent/mint/burn_coins validation interface * Add sent/mint/burn_coins validation interface * Revert naming to recv_packet_execute * add clarifying comment * Fix send/mint/burn_coins_validate comment * more clarifying comments * clippy --------- Co-authored-by: Philippe Laferriere <plafer@protonmail.com>
Closes: #502
Remark
I attempted the separation of the
recv_packet
function intorecv_packet_validate
andrecv_packet_execute
, but encountered a limitation to do so, because:Even if the validation fails, we still need to perform some execution logic. Specifically, we need to store a failure
acknowledgement
containing the error log and emit the subsequent event. The problem is that we don't have mutable access to the context in the validation step, which makes it tricky to properly implement this logic.There might be a way to solve this, but, IMO, highly likely requires us to touch some other parts and make changes beyond the scope of the current PR. Thus, I decided to put any ideas around this on hold, and that's the implementation I could come up with for now.
PR author checklist:
unclog
.docs/
).Reviewer checklist:
Files changed
in the GitHub PR explorer.