Switch from callcode to delegatecall
#222
Merged
Conversation
This file contains 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
kevincheng96
commented
Sep 27, 2024
| @@ -137,32 +137,3 @@ sequenceDiagram title: Execute Quark Operation with Callback | |||
| S -->> S: [14] Run Script | |||
| S -->> T: [15] Erc20 Transfer "Repay" [sender=User Wallet] | |||
| ``` | |||
|
|
|||
| ## Upgrade Quark Wallet | |||
There was a problem hiding this comment.
Interesting to see this. I guess there was a point in time when we were using a fully upgradeable proxy for the Quark wallet.
fluffywaffles
approved these changes
Sep 27, 2024
kevincheng96
force-pushed
the
kevin/delegatecall
branch
from
bffc5ee to
8e664aa
Compare
hayesgm
approved these changes
Oct 15, 2024
| // Note: CALLCODE is used to set the QuarkWallet as the `msg.sender` | ||
| success := | ||
| callcode(gas(), scriptAddress, /* value */ 0, add(scriptCalldata, 0x20), scriptCalldataLen, 0x0, 0) | ||
| success := delegatecall(gas(), scriptAddress, add(scriptCalldata, 0x20), scriptCalldataLen, 0x0, 0) |
There was a problem hiding this comment.
To be clear, we don't want to use msg.value here still, right?
There was a problem hiding this comment.
msg.value will always be 0 because none of the executeQuarkOperation functions are payable.
kevincheng96
force-pushed
the
kevin/delegatecall
branch
from
8e664aa to
09f8e8d
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Now that we use
tstore/tloadfor reentrancy guards, theonlyWalletmodifier is no longer needed. This is was pretty much the only remaining reason for usingcallcodeoverdelegatecallto execute a Quark script. Sincecallcodeis a deprecated opcode, it is better to switch over todelegatecall.The one side effect of this switch-over is that scripts have to use
address(this)instead ofmsg.senderto refer to the Quark wallet's address. Surprisingly, all our existing core scripts were already usingaddress(this), so the only changes we have to make are to the test scripts.