Revert use of uint8 everywhere except Safe contract

safe-global · Jan 10, 2024 · 9b7697a · 9b7697a
1 parent 85f043f
commit 9b7697a
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 15 deletions.
diff --git a/certora/harnesses/SafeHarness.sol b/certora/harnesses/SafeHarness.sol
@@ -16,18 +16,18 @@ contract SafeHarness is Safe {
     }
 
     // harnessed functions
-    function signatureSplitPublic(bytes memory signatures, uint256 pos) public pure returns (uint256 v, bytes32 r, bytes32 s) {
+    function signatureSplitPublic(bytes memory signatures, uint256 pos) public pure returns (uint8 v, bytes32 r, bytes32 s) {
         require(signatures.length >= 65 * (pos + 1));
         return signatureSplit(signatures, pos);
     }
 
-    function getCurrentOwner(bytes32 dataHash, uint256 v, bytes32 r, bytes32 s) public pure returns (address currentOwner) {
+    function getCurrentOwner(bytes32 dataHash, uint8 v, bytes32 r, bytes32 s) public pure returns (address currentOwner) {
         if (v == 0 || v == 1) {
             currentOwner = address(uint160(uint256(r)));
         } else if (v > 30) {
-            currentOwner = ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), uint8(v - 4), r, s);
+            currentOwner = ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s);
         } else {
-            currentOwner = ecrecover(dataHash, uint8(v), r, s);
+            currentOwner = ecrecover(dataHash, v, r, s);
         }
     }
 

diff --git a/certora/specs/Signatures.spec b/certora/specs/Signatures.spec
@@ -4,13 +4,13 @@ methods {
     function isOwner(address) external returns (bool) envfree;
 
     // harnessed
-    function signatureSplitPublic(bytes,uint256) external returns (uint256,bytes32,bytes32) envfree;
-    function getCurrentOwner(bytes32, uint256, bytes32, bytes32) external returns (address) envfree;
+    function signatureSplitPublic(bytes,uint256) external returns (uint8,bytes32,bytes32) envfree;
+    function getCurrentOwner(bytes32, uint8, bytes32, bytes32) external returns (address) envfree;
     function getTransactionHashPublic(address, uint256, bytes, Enum.Operation, uint256, uint256, uint256, address, address, uint256) external returns (bytes32) envfree;
     // needed for the getTransactionHash ghost for the execTransaction <> signatures rule
 
     // summaries
-    function SignatureDecoder.signatureSplit(bytes memory signatures, uint256 pos) internal returns (uint256,bytes32,bytes32) => signatureSplitGhost(signatures,pos);
+    function SignatureDecoder.signatureSplit(bytes memory signatures, uint256 pos) internal returns (uint8,bytes32,bytes32) => signatureSplitGhost(signatures,pos);
     function Safe.checkContractSignature(address, bytes32, bytes memory, uint256) internal => NONDET;
     // needed for the execTransaction <> signatures rule
     function Safe.getTransactionHash(
@@ -33,7 +33,7 @@ methods {
 
 definition MAX_UINT256() returns uint256 = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff;
 
-ghost mapping(bytes => mapping(uint256 => uint256)) mySigSplitV;
+ghost mapping(bytes => mapping(uint256 => uint8)) mySigSplitV;
 ghost mapping(bytes => mapping(uint256 => bytes32)) mySigSplitR;
 ghost mapping(bytes => mapping(uint256 => bytes32)) mySigSplitS;
 
@@ -50,7 +50,7 @@ ghost transactionHashGhost(
         address /*refundReceiver*/,
         uint256 /*_nonce*/ ) returns bytes32 ;
 
-function signatureSplitGhost(bytes signatures, uint256 pos) returns (uint256,bytes32,bytes32) {
+function signatureSplitGhost(bytes signatures, uint256 pos) returns (uint8,bytes32,bytes32) {
     return (mySigSplitV[signatures][pos], mySigSplitR[signatures][pos], mySigSplitS[signatures][pos]);
 }
 
@@ -63,10 +63,10 @@ rule checkSignatures() {
     bytes signaturesAB;
     bytes signaturesA;
     bytes signaturesB;
-    uint256 vA; bytes32 rA; bytes32 sA;
-    uint256 vB; bytes32 rB; bytes32 sB;
-    uint256 vAB1; bytes32 rAB1; bytes32 sAB1;
-    uint256 vAB2; bytes32 rAB2; bytes32 sAB2;
+    uint8 vA; bytes32 rA; bytes32 sA;
+    uint8 vB; bytes32 rB; bytes32 sB;
+    uint8 vAB1; bytes32 rAB1; bytes32 sAB1;
+    uint8 vAB2; bytes32 rAB2; bytes32 sAB2;
     vA, rA, sA = signatureSplitPublic(signaturesA, 0);
     vB, rB, sB = signatureSplitPublic(signaturesB, 0);
     vAB1, rAB1, sAB1 = signatureSplitPublic(signaturesAB, 0);

diff --git a/contracts/Safe.sol b/contracts/Safe.sol
@@ -313,7 +313,7 @@ contract Safe is
         // There cannot be an owner with address 0.
         address lastOwner = address(0);
         address currentOwner;
-        uint256 v;
+        uint256 v; // Implicit conversion from uint8 to uint256 will be done for v received from signatureSplit(...).
         bytes32 r;
         bytes32 s;
         uint256 i;

diff --git a/contracts/common/SignatureDecoder.sol b/contracts/common/SignatureDecoder.sol
@@ -18,7 +18,7 @@ abstract contract SignatureDecoder {
      * @return r Output value r of the signature.
      * @return s Output value s of the signature.
      */
-    function signatureSplit(bytes memory signatures, uint256 pos) internal pure returns (uint256 v, bytes32 r, bytes32 s) {
+    function signatureSplit(bytes memory signatures, uint256 pos) internal pure returns (uint8 v, bytes32 r, bytes32 s) {
         /* solhint-disable no-inline-assembly */
         /// @solidity memory-safe-assembly
         assembly {