Skip to content

Commit

Permalink
Add test for edge case where cached script address differs from the Q…
Browse files Browse the repository at this point in the history
…uark Operation script (#183)

Weird edge-case that seems benign and scripts really have to go out of their way to achieve this. The right way to fix this is to cache the `scriptAddress` before calling `executeScriptWithNonceLock`, but that adds a ~15k gas overhead. Consider our implementation a conscious optimization at this point.
  • Loading branch information
kevincheng96 authored Mar 15, 2024
1 parent 8d25fdf commit b2ab0d6
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 0 deletions.
16 changes: 16 additions & 0 deletions test/lib/CallOtherScript.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.23;

import "quark-core/src/QuarkScript.sol";
import "quark-core/src/QuarkWallet.sol";

contract CallOtherScript is QuarkScript {
function call(uint96 nonce, address scriptAddress, bytes calldata scriptCalldata) public {
// Anti-pattern to clear replay first, but this is necessary to hit our edge case
allowReplay();
QuarkWallet(payable(address(this))).stateManager().setActiveNonceAndCallback(
nonce, scriptAddress, scriptCalldata
);
QuarkWallet(payable(address(this))).stateManager().setNonce(nonce);
}
}
33 changes: 33 additions & 0 deletions test/quark-core/QuarkWallet.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -688,6 +688,39 @@ contract QuarkWalletTest is Test {
assertEq(counter.number(), 3);
}

/* ===== caching unintended script address edge case ===== */

// Script A clears nonce -> Call script B -> Script B clears nonce -> Returns to script A -> Script A sets nonce -> Script B is cached
// Note: This is not desired behavior. It is a benign edge-case that would be gassy (~15k gas overhead) to prevent.
function testSingleNonceCanCallDifferentScriptsAndCacheNonTargetScript() public {
// gas: do not meter set-up
vm.pauseGasMetering();
bytes memory callOtherScript = new YulHelper().getCode("CallOtherScript.sol/CallOtherScript.json");
// We just need to call any script that allows replays (clears the nonce)
bytes memory allowReplay = new YulHelper().getCode("AllowCallbacks.sol/AllowCallbacks.json");
address allowReplayAddress = codeJar.saveCode(allowReplay);
uint96 nonce = stateManager.nextNonce(address(aliceWallet));
QuarkWallet.QuarkOperation memory op = new QuarkOperationHelper().newBasicOpWithCalldata(
aliceWallet,
callOtherScript,
abi.encodeWithSignature(
"call(uint96,address,bytes)",
nonce,
allowReplayAddress,
abi.encodeWithSignature("allowCallbackAndReplay()")
),
ScriptType.ScriptAddress
);
(uint8 v, bytes32 r, bytes32 s) = new SignatureHelper().signOp(alicePrivateKey, aliceWallet, op);

// gas: meter execute
vm.resumeGasMetering();
aliceWallet.executeQuarkOperation(op, v, r, s);

// The cached script address is different from the script address in the Quark Operation
assertEq(stateManager.nonceScriptAddress(address(aliceWallet), nonce), allowReplayAddress);
}

/* ===== execution on Precompiles ===== */
// Quark is no longer able call precompiles directly due to empty code check, so these tests are commented out

Expand Down

0 comments on commit b2ab0d6

Please sign in to comment.