Skip to content
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

TStore for Transients #214

Merged
merged 3 commits into from
Sep 12, 2024
Merged

TStore for Transients #214

merged 3 commits into from
Sep 12, 2024

Conversation

hayesgm
Copy link
Contributor

@hayesgm hayesgm commented Sep 12, 2024

This patch bumps us up to 0.8.27 and cancun and starts using tstore and tload for transient reads and writes. Note: we still clear values mostly to ensure that post-nested calls the script will see 0 instead of the nested's own value.

Note: I have not taken a stab at understanding ALLOW_CALLBACKS and how that should function.

This patch bumps us up to 0.8.27 and cancun and starts using `tstore` and `tload` for transient reads and writes. Note: we still clear values mostly to ensure that post-nested calls the script will see 0 instead of the nested's own value.

Note: I have not taken a stab at understanding `ALLOW_CALLBACKS` and how that should function.

Patches:
  * Try to build allow callbacks to ensure it still works as expected, though it can't be observed now.
Copy link
Collaborator

@kevincheng96 kevincheng96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet, can't wait to see the gas savings from these changes 🙌

src/quark-core/src/QuarkScript.sol Show resolved Hide resolved
src/quark-core/src/QuarkScript.sol Show resolved Hide resolved
@@ -482,31 +482,28 @@ contract QuarkWallet is IERC1271 {
assembly {
// TODO: TSTORE the callback slot to 0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can address this TODO by also setting the callback slot to 0 after the callcode.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious if there's a big benefit here, since tstore will clear naturally? I guess all of these clears are mostly in the camp of "if you do nesting, you're 8% safer"?

test/quark-core/Callbacks.t.sol Outdated Show resolved Hide resolved
This patch simply adds a cancel core script that can be used (a) as a
nop to cancel a chain, or (b) to directly call
`nonceManager.cancel(nonce)` with one or more nonces. These are merely
given as helpers for the end-users.
@hayesgm hayesgm merged commit d24faf0 into hayesgm/cancel-by-new-op Sep 12, 2024
2 of 4 checks passed
@hayesgm hayesgm deleted the hayesgm/tstore branch September 12, 2024 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants