From 7a807c0ec9eac866eea3be55e87ec84b0dd280ce Mon Sep 17 00:00:00 2001 From: katzman Date: Fri, 29 Mar 2024 15:44:07 -0700 Subject: [PATCH 1/2] Add clearer messaging around the risks of overwriting _getNonce --- script/universal/MultisigBase.sol | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/script/universal/MultisigBase.sol b/script/universal/MultisigBase.sol index 193c100..bc47ed3 100644 --- a/script/universal/MultisigBase.sol +++ b/script/universal/MultisigBase.sol @@ -21,6 +21,12 @@ abstract contract MultisigBase is Simulator { // Virtual method which can be overwritten // Default logic here is vestigial for backwards compatibility + // IMPORTANT: this method is used in the sign, simulate, AND execution contexts + // If you override it, ensure that the behavior is correct for all contexts + // As an example, if you are pre-signing a message that needs safe.nonce+1 (before safe.nonce is executed), + // you should override this method and then explicitly set the nonce value with an env var. + // Overwriting this method with safe.nonce + 1 will cause issues upon execution because the transaction + // hash will differ from the one signed. function _getNonce(IGnosisSafe safe) internal view virtual returns (uint256 nonce) { nonce = safe.nonce(); console.log("Safe current nonce:", nonce); From aa0404bd8e4a253d9630e37caedcdc16d4c8056b Mon Sep 17 00:00:00 2001 From: katzman Date: Mon, 1 Apr 2024 10:45:44 -0700 Subject: [PATCH 2/2] Update MultisigBase.sol Improve clarity of comment --- script/universal/MultisigBase.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/script/universal/MultisigBase.sol b/script/universal/MultisigBase.sol index bc47ed3..853fb74 100644 --- a/script/universal/MultisigBase.sol +++ b/script/universal/MultisigBase.sol @@ -24,7 +24,7 @@ abstract contract MultisigBase is Simulator { // IMPORTANT: this method is used in the sign, simulate, AND execution contexts // If you override it, ensure that the behavior is correct for all contexts // As an example, if you are pre-signing a message that needs safe.nonce+1 (before safe.nonce is executed), - // you should override this method and then explicitly set the nonce value with an env var. + // you should explicitly set the nonce value with an env var. // Overwriting this method with safe.nonce + 1 will cause issues upon execution because the transaction // hash will differ from the one signed. function _getNonce(IGnosisSafe safe) internal view virtual returns (uint256 nonce) {