Conversation
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! I left just a few questions/comments inline.
| #! This procedure does not verify the caller. Wrap with access control in the account component | ||
| #! composition if only privileged accounts should pause. |
There was a problem hiding this comment.
This will be changed in a follow-up PR, right? (i.e., we'll have owner-based or RBAC-based checks)
| #! Invocation: exec | ||
| pub proc assert_not_paused |
There was a problem hiding this comment.
Not related to this PR, but we now technically have two types of procedures exposed by a component:
- Procedures that form the public interface - these procedures are the ones included in the account's code commitment and they must be invoked using the
callinstruction. - Procedures that can be invoked by other components deployed in the account - usually, via the
execinstruction.
The above distinction doesn't matter too much for the components that just re-export functionality from the standard library, but for non-standardized components it would make a difference. And so, it may be a good idea to make this destination explicity.
One way to do it is by switching to attributes to identify exported procedures. This would follow the approach we've taken with note scripts (and soon will take with transaction scripts). For example, we could have @interface(external) and @interface(internal) attributes to identify these two types - though, better naming would probably be needed.
cc @mmagician, @greenhat, @bitwalker in case you guys have thoughts on this.
| pub proc on_before_asset_added_to_account | ||
| exec.assert_not_paused | ||
| # => [ASSET_KEY, ASSET_VALUE, pad(8)] | ||
|
|
||
| dropw | ||
| # => [ASSET_VALUE, pad(12)] | ||
| end | ||
|
|
||
| #! Callback when a callbacks-enabled asset is added to an output note. | ||
| #! | ||
| #! Panics if this faucet account is paused. | ||
| #! | ||
| #! Inputs: [ASSET_KEY, ASSET_VALUE, note_idx, pad(7)] | ||
| #! Outputs: [ASSET_VALUE, pad(12)] | ||
| #! | ||
| #! Invocation: call | ||
| pub proc on_before_asset_added_to_note |
There was a problem hiding this comment.
Why do we need these callbacks here? I was imagining that they'd be defined in more specific contexts (e.g., in regulated stablecoin).
| /// Component library path (merged account module name). | ||
| pub const NAME: &'static str = "miden::standards::components::utils::pausable"; |
There was a problem hiding this comment.
Similar to my comments in another PR: I don't think we should include components in component name. I think this could probably be just:
miden::standards::utils::pausable
Though, if we find something more descriptive than just utils, that would be better.
In either case, we should make sure that slot and component names are consistent - i.e., component name is a prefix of the slot names within that component.
2 participants
Closes issue including the design discussion provided here: #2241
This PR adds a Pausable component without Access Control mechanism included as we split this work into small PRs.
In the next PRs, we'll integrate the following features:
Related Issues: