-
Notifications
You must be signed in to change notification settings - Fork 164
feat(protocol): tie the asset callback flag to the callback slots #3658
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
Merged
Merged
Changes from 13 commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
671eeea
feat(protocol): derive the asset callback flag from installed callbac…
onurinanc 172424b
feat(protocol): require an enabled asset callback flag when a callbac…
onurinanc ebca364
docs(protocol): record the callback slot invariant in the account upg…
onurinanc c78a467
refactor: drop manual asset callback flag wiring from account factories
onurinanc a184449
chore: add changelog entry for the asset callback slot invariant
onurinanc 396da34
changelog
onurinanc 6ed08a6
remove comments
onurinanc 4806c4e
refactor(protocol): keep the builder's asset callback field typed as …
onurinanc 53e5f10
fix comments
onurinanc e4fa0b6
fix comments
onurinanc 4ae9bbd
Merge remote-tracking branch 'origin' into refactor-callback-fix
onurinanc 708a1b2
merge
onurinanc bcff8f8
refactor(protocol): address review comments on the asset callback inv…
onurinanc 912ef1d
docs(standards): document why TokenPolicyManager installs callback sl…
onurinanc 434685a
test: simplify the callback flag prologue test to a full transaction …
onurinanc 83a7f69
docs(protocol): separate asset callback dispatch from callback presence
onurinanc e5db4dd
docs(standards): drop the AccountBuilder implementation detail from t…
onurinanc baf1ffc
fix(agglayer): require the reserved asset callback slots on AggLayer …
onurinanc 78a1ce1
Merge remote-tracking branch 'origin' into refactor-callback-fix
onurinanc 10e46b2
refactor(protocol): rename AccountStorage::has_callbacks to has_callb…
onurinanc File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -224,6 +224,10 @@ end | |
| pub proc account_upgrade | ||
| # TODO(code_upgrades): Account upgrades must ensure the same conditions hold for an upgraded | ||
| # account as validated in account::{validate_storage, validate_procedures}. | ||
| # The same applies to the asset callback rule validated in | ||
| # prologue::validate_asset_callbacks: an upgrade must not add an asset callback slot to an | ||
| # account whose asset callback flag is disabled, since the flag is immutable and the callback | ||
| # could then never be invoked. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for adding this 👌
mmagician marked this conversation as resolved.
Outdated
|
||
| # check that this procedure was executed against the native account | ||
| exec.memory::assert_native_account | ||
| # => [CODE_UPGRADE_COMMITMENT, STORAGE_UPGRADE_COMMITMENT, pad(8)] | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I think not requiring slots when the account ID flag is enabled actually makes sense. This retains the ability to add new types of callback slots that are introduced in a later protocol version.
I'd add something like this here: