Commit 148cb966 authored by ben-chain's avatar ben-chain Committed by GitHub

remove msg.sender from all SafeEM calls (#68)

parent d85d00fa
...@@ -45,7 +45,6 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount { ...@@ -45,7 +45,6 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
bytes memory _returndata bytes memory _returndata
) )
{ {
address ovmExecutionManager = msg.sender;
bool isEthSign = _signatureType == Lib_OVMCodec.EOASignatureType.ETH_SIGNED_MESSAGE; bool isEthSign = _signatureType == Lib_OVMCodec.EOASignatureType.ETH_SIGNED_MESSAGE;
// Address of this contract within the ovm (ovmADDRESS) should be the same as the // Address of this contract within the ovm (ovmADDRESS) should be the same as the
...@@ -53,14 +52,13 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount { ...@@ -53,14 +52,13 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
// account abstraction even though the user isn't a contract. // account abstraction even though the user isn't a contract.
// Need to make sure that the transaction nonce is right and bump it if so. // Need to make sure that the transaction nonce is right and bump it if so.
Lib_SafeExecutionManagerWrapper.safeREQUIRE( Lib_SafeExecutionManagerWrapper.safeREQUIRE(
msg.sender,
Lib_ECDSAUtils.recover( Lib_ECDSAUtils.recover(
_transaction, _transaction,
isEthSign, isEthSign,
_v, _v,
_r, _r,
_s _s
) == Lib_SafeExecutionManagerWrapper.safeADDRESS(ovmExecutionManager), ) == Lib_SafeExecutionManagerWrapper.safeADDRESS(),
"Signature provided for EOA transaction execution is invalid." "Signature provided for EOA transaction execution is invalid."
); );
...@@ -68,16 +66,14 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount { ...@@ -68,16 +66,14 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
// Need to make sure that the transaction nonce is right. // Need to make sure that the transaction nonce is right.
Lib_SafeExecutionManagerWrapper.safeREQUIRE( Lib_SafeExecutionManagerWrapper.safeREQUIRE(
msg.sender, decodedTx.nonce == Lib_SafeExecutionManagerWrapper.safeGETNONCE(),
decodedTx.nonce == Lib_SafeExecutionManagerWrapper.safeGETNONCE(ovmExecutionManager),
"Transaction nonce does not match the expected nonce." "Transaction nonce does not match the expected nonce."
); );
// Transfer fee to relayer. // Transfer fee to relayer.
address relayer = Lib_SafeExecutionManagerWrapper.safeCALLER(ovmExecutionManager); address relayer = Lib_SafeExecutionManagerWrapper.safeCALLER();
uint256 fee = decodedTx.gasLimit * decodedTx.gasPrice; uint256 fee = decodedTx.gasLimit * decodedTx.gasPrice;
Lib_SafeExecutionManagerWrapper.safeCALL( Lib_SafeExecutionManagerWrapper.safeCALL(
ovmExecutionManager,
gasleft(), gasleft(),
ETH_ERC20_ADDRESS, ETH_ERC20_ADDRESS,
abi.encodeWithSignature("transfer(address,uint256)", relayer, fee) abi.encodeWithSignature("transfer(address,uint256)", relayer, fee)
...@@ -86,7 +82,6 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount { ...@@ -86,7 +82,6 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
// Contract creations are signalled by sending a transaction to the zero address. // Contract creations are signalled by sending a transaction to the zero address.
if (decodedTx.to == address(0)) { if (decodedTx.to == address(0)) {
address created = Lib_SafeExecutionManagerWrapper.safeCREATE( address created = Lib_SafeExecutionManagerWrapper.safeCREATE(
ovmExecutionManager,
decodedTx.gasLimit - 2000, decodedTx.gasLimit - 2000,
decodedTx.data decodedTx.data
); );
...@@ -98,10 +93,9 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount { ...@@ -98,10 +93,9 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
// We only want to bump the nonce for `ovmCALL` because `ovmCREATE` automatically bumps // We only want to bump the nonce for `ovmCALL` because `ovmCREATE` automatically bumps
// the nonce of the calling account. Normally an EOA would bump the nonce for both // the nonce of the calling account. Normally an EOA would bump the nonce for both
// cases, but since this is a contract we'd end up bumping the nonce twice. // cases, but since this is a contract we'd end up bumping the nonce twice.
Lib_SafeExecutionManagerWrapper.safeSETNONCE(ovmExecutionManager, decodedTx.nonce + 1); Lib_SafeExecutionManagerWrapper.safeSETNONCE(decodedTx.nonce + 1);
return Lib_SafeExecutionManagerWrapper.safeCALL( return Lib_SafeExecutionManagerWrapper.safeCALL(
ovmExecutionManager,
decodedTx.gasLimit, decodedTx.gasLimit,
decodedTx.to, decodedTx.to,
decodedTx.data decodedTx.data
......
...@@ -30,7 +30,6 @@ contract OVM_ProxyEOA { ...@@ -30,7 +30,6 @@ contract OVM_ProxyEOA {
external external
{ {
(bool success, bytes memory returndata) = Lib_SafeExecutionManagerWrapper.safeDELEGATECALL( (bool success, bytes memory returndata) = Lib_SafeExecutionManagerWrapper.safeDELEGATECALL(
msg.sender,
gasleft(), gasleft(),
getImplementation(), getImplementation(),
msg.data msg.data
...@@ -42,7 +41,6 @@ contract OVM_ProxyEOA { ...@@ -42,7 +41,6 @@ contract OVM_ProxyEOA {
} }
} else { } else {
Lib_SafeExecutionManagerWrapper.safeREVERT( Lib_SafeExecutionManagerWrapper.safeREVERT(
msg.sender,
string(returndata) string(returndata)
); );
} }
...@@ -59,8 +57,7 @@ contract OVM_ProxyEOA { ...@@ -59,8 +57,7 @@ contract OVM_ProxyEOA {
external external
{ {
Lib_SafeExecutionManagerWrapper.safeREQUIRE( Lib_SafeExecutionManagerWrapper.safeREQUIRE(
msg.sender, Lib_SafeExecutionManagerWrapper.safeADDRESS() == Lib_SafeExecutionManagerWrapper.safeCALLER(),
Lib_SafeExecutionManagerWrapper.safeADDRESS(msg.sender) == Lib_SafeExecutionManagerWrapper.safeCALLER(msg.sender),
"EOAs can only upgrade their own EOA implementation" "EOAs can only upgrade their own EOA implementation"
); );
...@@ -75,7 +72,6 @@ contract OVM_ProxyEOA { ...@@ -75,7 +72,6 @@ contract OVM_ProxyEOA {
{ {
return address(uint160(uint256( return address(uint160(uint256(
Lib_SafeExecutionManagerWrapper.safeSLOAD( Lib_SafeExecutionManagerWrapper.safeSLOAD(
msg.sender,
bytes32(uint256(0)) bytes32(uint256(0))
) )
))); )));
...@@ -91,7 +87,6 @@ contract OVM_ProxyEOA { ...@@ -91,7 +87,6 @@ contract OVM_ProxyEOA {
internal internal
{ {
Lib_SafeExecutionManagerWrapper.safeSSTORE( Lib_SafeExecutionManagerWrapper.safeSSTORE(
msg.sender,
bytes32(uint256(0)), bytes32(uint256(0)),
bytes32(uint256(uint160(_implementation))) bytes32(uint256(uint160(_implementation)))
); );
......
...@@ -16,7 +16,6 @@ contract OVM_ProxySequencerEntrypoint { ...@@ -16,7 +16,6 @@ contract OVM_ProxySequencerEntrypoint {
external external
{ {
Lib_SafeExecutionManagerWrapper.safeDELEGATECALL( Lib_SafeExecutionManagerWrapper.safeDELEGATECALL(
msg.sender,
gasleft(), gasleft(),
_getImplementation(), _getImplementation(),
msg.data msg.data
...@@ -35,7 +34,6 @@ contract OVM_ProxySequencerEntrypoint { ...@@ -35,7 +34,6 @@ contract OVM_ProxySequencerEntrypoint {
external external
{ {
Lib_SafeExecutionManagerWrapper.safeREQUIRE( Lib_SafeExecutionManagerWrapper.safeREQUIRE(
msg.sender,
_getOwner() == address(0), _getOwner() == address(0),
"ProxySequencerEntrypoint has already been inited" "ProxySequencerEntrypoint has already been inited"
); );
...@@ -49,8 +47,7 @@ contract OVM_ProxySequencerEntrypoint { ...@@ -49,8 +47,7 @@ contract OVM_ProxySequencerEntrypoint {
external external
{ {
Lib_SafeExecutionManagerWrapper.safeREQUIRE( Lib_SafeExecutionManagerWrapper.safeREQUIRE(
msg.sender, _getOwner() == Lib_SafeExecutionManagerWrapper.safeCALLER(),
_getOwner() == Lib_SafeExecutionManagerWrapper.safeCALLER(msg.sender),
"Only owner can upgrade the Entrypoint" "Only owner can upgrade the Entrypoint"
); );
...@@ -68,7 +65,6 @@ contract OVM_ProxySequencerEntrypoint { ...@@ -68,7 +65,6 @@ contract OVM_ProxySequencerEntrypoint {
internal internal
{ {
Lib_SafeExecutionManagerWrapper.safeSSTORE( Lib_SafeExecutionManagerWrapper.safeSSTORE(
msg.sender,
bytes32(uint256(0)), bytes32(uint256(0)),
bytes32(uint256(uint160(_implementation))) bytes32(uint256(uint160(_implementation)))
); );
...@@ -82,7 +78,6 @@ contract OVM_ProxySequencerEntrypoint { ...@@ -82,7 +78,6 @@ contract OVM_ProxySequencerEntrypoint {
{ {
return address(uint160(uint256( return address(uint160(uint256(
Lib_SafeExecutionManagerWrapper.safeSLOAD( Lib_SafeExecutionManagerWrapper.safeSLOAD(
msg.sender,
bytes32(uint256(0)) bytes32(uint256(0))
) )
))); )));
...@@ -94,7 +89,6 @@ contract OVM_ProxySequencerEntrypoint { ...@@ -94,7 +89,6 @@ contract OVM_ProxySequencerEntrypoint {
internal internal
{ {
Lib_SafeExecutionManagerWrapper.safeSSTORE( Lib_SafeExecutionManagerWrapper.safeSSTORE(
msg.sender,
bytes32(uint256(1)), bytes32(uint256(1)),
bytes32(uint256(uint160(_owner))) bytes32(uint256(uint160(_owner)))
); );
...@@ -108,7 +102,6 @@ contract OVM_ProxySequencerEntrypoint { ...@@ -108,7 +102,6 @@ contract OVM_ProxySequencerEntrypoint {
{ {
return address(uint160(uint256( return address(uint160(uint256(
Lib_SafeExecutionManagerWrapper.safeSLOAD( Lib_SafeExecutionManagerWrapper.safeSLOAD(
msg.sender,
bytes32(uint256(1)) bytes32(uint256(1))
) )
))); )));
......
...@@ -63,10 +63,10 @@ contract OVM_SequencerEntrypoint { ...@@ -63,10 +63,10 @@ contract OVM_SequencerEntrypoint {
s s
); );
if (Lib_SafeExecutionManagerWrapper.safeEXTCODESIZE(msg.sender, target) == 0) { if (Lib_SafeExecutionManagerWrapper.safeEXTCODESIZE(target) == 0) {
// ProxyEOA has not yet been deployed for this EOA. // ProxyEOA has not yet been deployed for this EOA.
bytes32 messageHash = Lib_ECDSAUtils.getMessageHash(encodedTx, isEthSignedMessage); bytes32 messageHash = Lib_ECDSAUtils.getMessageHash(encodedTx, isEthSignedMessage);
Lib_SafeExecutionManagerWrapper.safeCREATEEOA(msg.sender, messageHash, uint8(v), r, s); Lib_SafeExecutionManagerWrapper.safeCREATEEOA(messageHash, uint8(v), r, s);
} }
// ProxyEOA has been deployed for this EOA, continue to CALL. // ProxyEOA has been deployed for this EOA, continue to CALL.
...@@ -80,7 +80,6 @@ contract OVM_SequencerEntrypoint { ...@@ -80,7 +80,6 @@ contract OVM_SequencerEntrypoint {
); );
Lib_SafeExecutionManagerWrapper.safeCALL( Lib_SafeExecutionManagerWrapper.safeCALL(
msg.sender,
gasleft(), gasleft(),
target, target,
callbytes callbytes
...@@ -111,7 +110,6 @@ contract OVM_SequencerEntrypoint { ...@@ -111,7 +110,6 @@ contract OVM_SequencerEntrypoint {
return TransactionType.ETH_SIGNED_MESSAGE; return TransactionType.ETH_SIGNED_MESSAGE;
} else { } else {
Lib_SafeExecutionManagerWrapper.safeREVERT( Lib_SafeExecutionManagerWrapper.safeREVERT(
msg.sender,
"Transaction type must be 0 or 2" "Transaction type must be 0 or 2"
); );
} }
......
...@@ -43,21 +43,18 @@ contract mockOVM_ECDSAContractAccount is iOVM_ECDSAContractAccount { ...@@ -43,21 +43,18 @@ contract mockOVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
bytes memory _returndata bytes memory _returndata
) )
{ {
address ovmExecutionManager = msg.sender;
bool isEthSign = _signatureType == Lib_OVMCodec.EOASignatureType.ETH_SIGNED_MESSAGE; bool isEthSign = _signatureType == Lib_OVMCodec.EOASignatureType.ETH_SIGNED_MESSAGE;
Lib_OVMCodec.EIP155Transaction memory decodedTx = Lib_OVMCodec.decodeEIP155Transaction(_transaction, isEthSign); Lib_OVMCodec.EIP155Transaction memory decodedTx = Lib_OVMCodec.decodeEIP155Transaction(_transaction, isEthSign);
// Need to make sure that the transaction nonce is right. // Need to make sure that the transaction nonce is right.
Lib_SafeExecutionManagerWrapper.safeREQUIRE( Lib_SafeExecutionManagerWrapper.safeREQUIRE(
msg.sender, decodedTx.nonce == Lib_SafeExecutionManagerWrapper.safeGETNONCE(),
decodedTx.nonce == Lib_SafeExecutionManagerWrapper.safeGETNONCE(ovmExecutionManager),
"Transaction nonce does not match the expected nonce." "Transaction nonce does not match the expected nonce."
); );
// Contract creations are signalled by sending a transaction to the zero address. // Contract creations are signalled by sending a transaction to the zero address.
if (decodedTx.to == address(0)) { if (decodedTx.to == address(0)) {
address created = Lib_SafeExecutionManagerWrapper.safeCREATE( address created = Lib_SafeExecutionManagerWrapper.safeCREATE(
ovmExecutionManager,
decodedTx.gasLimit, decodedTx.gasLimit,
decodedTx.data decodedTx.data
); );
...@@ -68,10 +65,9 @@ contract mockOVM_ECDSAContractAccount is iOVM_ECDSAContractAccount { ...@@ -68,10 +65,9 @@ contract mockOVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
// We only want to bump the nonce for `ovmCALL` because `ovmCREATE` automatically bumps // We only want to bump the nonce for `ovmCALL` because `ovmCREATE` automatically bumps
// the nonce of the calling account. Normally an EOA would bump the nonce for both // the nonce of the calling account. Normally an EOA would bump the nonce for both
// cases, but since this is a contract we'd end up bumping the nonce twice. // cases, but since this is a contract we'd end up bumping the nonce twice.
Lib_SafeExecutionManagerWrapper.safeSETNONCE(ovmExecutionManager, decodedTx.nonce + 1); Lib_SafeExecutionManagerWrapper.safeSETNONCE(decodedTx.nonce + 1);
return Lib_SafeExecutionManagerWrapper.safeCALL( return Lib_SafeExecutionManagerWrapper.safeCALL(
ovmExecutionManager,
decodedTx.gasLimit, decodedTx.gasLimit,
decodedTx.to, decodedTx.to,
decodedTx.data decodedTx.data
...@@ -91,7 +87,6 @@ contract mockOVM_ECDSAContractAccount is iOVM_ECDSAContractAccount { ...@@ -91,7 +87,6 @@ contract mockOVM_ECDSAContractAccount is iOVM_ECDSAContractAccount {
) )
{ {
return Lib_SafeExecutionManagerWrapper.safeCALL( return Lib_SafeExecutionManagerWrapper.safeCALL(
msg.sender,
_gasLimit, _gasLimit,
_to, _to,
_data _data
......
...@@ -65,7 +65,7 @@ const test_ovmCREATEEOA: TestDefinition = { ...@@ -65,7 +65,7 @@ const test_ovmCREATEEOA: TestDefinition = {
address: '0x17ec8597ff92C3F44523bDc65BF0f1bE632917ff', address: '0x17ec8597ff92C3F44523bDc65BF0f1bE632917ff',
}, },
expectedReturnStatus: true, expectedReturnStatus: true,
expectedReturnValue: 1638, expectedReturnValue: 1619,
}, },
], ],
}, },
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment