Skip to content

Commit

Permalink
Set callback slot to 0 before executing script (#221)
Browse files Browse the repository at this point in the history
We need to set the callback slot to 0 before executing a script to
ensure that nested operations don't inherit the callback value of the
parent operation.
  • Loading branch information
kevincheng96 authored Sep 30, 2024
1 parent 986d547 commit 0123c18
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/quark-core/src/QuarkWallet.sol
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,9 @@ contract QuarkWallet is IERC1271 {
// Transiently store the active submission token
tstore(activeSubmissionTokenSlot, submissionToken)

// Transiently set the callback slot to 0
tstore(callbackSlot, 0)

// Note: CALLCODE is used to set the QuarkWallet as the `msg.sender`
success :=
callcode(gas(), scriptAddress, /* value */ 0, add(scriptCalldata, 0x20), scriptCalldataLen, 0x0, 0)
Expand Down
16 changes: 16 additions & 0 deletions test/lib/GetCallbackDetails.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// SPDX-License-Identifier: BSD-3-Clause
pragma solidity 0.8.27;

import {QuarkWalletMetadata} from "quark-core/src/QuarkWallet.sol";
import {QuarkScript} from "quark-core/src/QuarkScript.sol";

contract GetCallbackDetails is QuarkScript {
function getCallbackAddress() public view returns (address) {
bytes32 callbackSlot = QuarkWalletMetadata.CALLBACK_SLOT;
address callbackAddress;
assembly {
callbackAddress := tload(callbackSlot)
}
return callbackAddress;
}
}
35 changes: 35 additions & 0 deletions test/quark-core/Callbacks.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,44 @@ contract CallbacksTest is Test {
// gas: meter execute
vm.resumeGasMetering();
aliceWallet.executeQuarkOperation(parentOp, v, r, s);

assertEq(counter.number(), 11);
}

function testNestedCallbackResetsCallbackSlot() public {
// gas: do not meter set-up
vm.pauseGasMetering();
bytes memory getCallbackDetails = new YulHelper().getCode("GetCallbackDetails.sol/GetCallbackDetails.json");
bytes memory executeOtherScript =
new YulHelper().getCode("ExecuteOtherOperation.sol/ExecuteOtherOperation.json");

QuarkWallet.QuarkOperation memory nestedOp = new QuarkOperationHelper().newBasicOpWithCalldata(
aliceWallet, getCallbackDetails, abi.encodeWithSignature("getCallbackAddress()"), ScriptType.ScriptAddress
);

(uint8 v_, bytes32 r_, bytes32 s_) = new SignatureHelper().signOp(alicePrivateKey, aliceWallet, nestedOp);

QuarkWallet.QuarkOperation memory parentOp = new QuarkOperationHelper().newBasicOpWithCalldata(
aliceWallet,
executeOtherScript,
abi.encodeWithSelector(ExecuteOtherOperation.run.selector, nestedOp, v_, r_, s_),
ScriptType.ScriptAddress
);

parentOp.nonce = new QuarkOperationHelper().incrementNonce(nestedOp.nonce);

(uint8 v, bytes32 r, bytes32 s) = new SignatureHelper().signOp(alicePrivateKey, aliceWallet, parentOp);

// gas: meter execute
vm.resumeGasMetering();
bytes memory result = aliceWallet.executeQuarkOperation(parentOp, v, r, s);
// We decode twice because the result is encoded twice due to the nested operation
address innerCallbackAddress = abi.decode(abi.decode(result, (bytes)), (address));

// The inner callback address should be 0
assertEq(innerCallbackAddress, address(0));
}

function testNestedCallWithNoCallbackSucceeds() public {
// gas: do not meter set-up
vm.pauseGasMetering();
Expand Down

0 comments on commit 0123c18

Please sign in to comment.