diff --git a/contracts/exchange/contracts/examples/Validator.sol b/contracts/exchange/contracts/examples/Validator.sol index 44e36edf45..0c34fa2c62 100644 --- a/contracts/exchange/contracts/examples/Validator.sol +++ b/contracts/exchange/contracts/examples/Validator.sol @@ -39,7 +39,7 @@ contract Validator is /// @param hash Message hash that is signed. /// @param signerAddress Address that should have signed the given hash. /// @param signature Proof of signing. - /// @return Validity of signature. + /// @return Returns a known magic value if the signature is valid. // solhint-disable no-unused-vars function isValidSignature( bytes32 hash, @@ -51,8 +51,8 @@ contract Validator is returns (bytes4) { require(signerAddress == VALID_SIGNER, "INVALID_SIGNER"); - bytes4 magic_salt = bytes4(keccak256("isValidValidatorSignature(address,bytes32,address,bytes)")); - return magic_salt; + bytes4 magicValue = bytes4(keccak256("isValidValidatorSignature(address,bytes32,address,bytes)")); + return magicValue; } // solhint-enable no-unused-vars } diff --git a/contracts/exchange/contracts/examples/Wallet.sol b/contracts/exchange/contracts/examples/Wallet.sol index f75e5069ae..1a6f5ea499 100644 --- a/contracts/exchange/contracts/examples/Wallet.sol +++ b/contracts/exchange/contracts/examples/Wallet.sol @@ -41,7 +41,7 @@ contract Wallet is /// The signer must match the owner of this wallet. /// @param hash Message hash that is signed. /// @param eip712Signature Proof of signing. - /// @return Validity of signature. + /// @return Returns a known magic value if the signature is valid. function isValidSignature( bytes32 hash, bytes calldata eip712Signature @@ -60,7 +60,7 @@ contract Wallet is bytes32 s = eip712Signature.readBytes32(33); address recoveredAddress = ecrecover(hash, v, r, s); require(WALLET_OWNER == recoveredAddress, "INVALID_SIGNATURE"); - bytes4 magic_salt = bytes4(keccak256("isValidWalletSignature(bytes32,address,bytes)")); - return magic_salt; + bytes4 magicValue = bytes4(keccak256("isValidWalletSignature(bytes32,address,bytes)")); + return magicValue; } } diff --git a/contracts/exchange/contracts/src/MixinSignatureValidator.sol b/contracts/exchange/contracts/src/MixinSignatureValidator.sol index 17e5a98d6f..2cc4fd57ba 100644 --- a/contracts/exchange/contracts/src/MixinSignatureValidator.sol +++ b/contracts/exchange/contracts/src/MixinSignatureValidator.sol @@ -238,13 +238,13 @@ contract MixinSignatureValidator is internal view returns (bool isValid) - { + { bytes memory callData = abi.encodeWithSelector( IWallet(walletAddress).isValidSignature.selector, hash, signature ); - bytes32 magic_salt = bytes32(bytes4(keccak256("isValidWalletSignature(bytes32,address,bytes)"))); + bytes32 magicValue = bytes32(bytes4(keccak256("isValidWalletSignature(bytes32,address,bytes)"))); assembly { if iszero(extcodesize(walletAddress)) { // Revert with `Error("WALLET_ERROR")` @@ -287,7 +287,7 @@ contract MixinSignatureValidator is // Signature is valid if call did not revert and returned true isValid := eq( and(mload(cdStart), 0xffffffff00000000000000000000000000000000000000000000000000000000), - and(magic_salt, 0xffffffff00000000000000000000000000000000000000000000000000000000) + and(magicValue, 0xffffffff00000000000000000000000000000000000000000000000000000000) ) } } @@ -316,7 +316,7 @@ contract MixinSignatureValidator is signerAddress, signature ); - bytes32 magic_salt = bytes32(bytes4(keccak256("isValidValidatorSignature(address,bytes32,address,bytes)"))); + bytes32 magicValue = bytes32(bytes4(keccak256("isValidValidatorSignature(address,bytes32,address,bytes)"))); assembly { if iszero(extcodesize(validatorAddress)) { // Revert with `Error("VALIDATOR_ERROR")` @@ -359,7 +359,7 @@ contract MixinSignatureValidator is // Signature is valid if call did not revert and returned true isValid := eq( and(mload(cdStart), 0xffffffff00000000000000000000000000000000000000000000000000000000), - and(magic_salt, 0xffffffff00000000000000000000000000000000000000000000000000000000) + and(magicValue, 0xffffffff00000000000000000000000000000000000000000000000000000000) ) } } diff --git a/packages/contract-artifacts/CHANGELOG.json b/packages/contract-artifacts/CHANGELOG.json index 71ee909a94..f22b16c37a 100644 --- a/packages/contract-artifacts/CHANGELOG.json +++ b/packages/contract-artifacts/CHANGELOG.json @@ -1,4 +1,13 @@ [ + { + "version": "2.2.0", + "changes": [ + { + "note": "Update `IWallet` and `IValidator` to reflect Mainnet", + "pr": 2078 + } + ] + }, { "version": "2.1.0", "changes": [ diff --git a/packages/order-utils/CHANGELOG.json b/packages/order-utils/CHANGELOG.json index 22885c108b..ca3fff1527 100644 --- a/packages/order-utils/CHANGELOG.json +++ b/packages/order-utils/CHANGELOG.json @@ -1,4 +1,13 @@ [ + { + "version": "8.2.6", + "changes": [ + { + "note": "Fix `Wallet` and `Validator` signature validation", + "pr": 2078 + } + ] + }, { "timestamp": 1565296576, "version": "8.2.5",