pallet-assets callback improvement #13543
Conversation
dmitry-markin
added
A0-please_review
|
Can someone else also please take a look? 🙏 |
| status: AssetStatus::Live, | ||
| }, | ||
| ); | ||
| ensure!(T::CallbackHandle::created(&id, &owner).is_ok(), Error::<T, I>::CallbackFailed); |
There was a problem hiding this comment.
This relies on transactional stroage since the insert above happens after the failure here.
There was a problem hiding this comment.
This was intentional since my understanding is that this is the default behavior (also got this comment below).
I also thought it would be good to have Assets entry in the DB before the callback (just in case the data from it is needed for the callback).
Open to suggestions though 🙂
There was a problem hiding this comment.
you can use require_transactional to annotate it
substrate/frame/support/src/lib.rs
Line 657 in 959c5b6
kianenigma
left a comment
kianenigma
left a comment
There was a problem hiding this comment.
This is a step in the right direction. We should leverage transactional-by-default more and more.
|
bot merge |
6 participants
Improves
AssetsCallbacktrait by introducingResult<(), ()>as the return type.This allows callback to inform the pallet whether the asset creation (or destruction) should complete or not.
Also moves the
destroyedcallback invocation todo_finish_destroyfunction.