Commit 79ef2f02 authored by smartcontracts's avatar smartcontracts Committed by GitHub

fix: Make ovmSETNONCE monotonic (#312)

* fix: Make ovmSETNONCE monotonic

* Use ovmINCREMENTNONCE instead

* Fix build errors

* fix remaining build errors

* last commit was a lie
parent 015bcd94
...@@ -128,7 +128,7 @@ contract OVM_ECDSAContractAccount is iOVM_ECDSAContractAccount { ...@@ -128,7 +128,7 @@ 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(decodedTx.nonce + 1); Lib_SafeExecutionManagerWrapper.safeINCREMENTNONCE();
return Lib_SafeExecutionManagerWrapper.safeCALL( return Lib_SafeExecutionManagerWrapper.safeCALL(
decodedTx.gasLimit, decodedTx.gasLimit,
......
...@@ -459,17 +459,20 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver { ...@@ -459,17 +459,20 @@ contract OVM_ExecutionManager is iOVM_ExecutionManager, Lib_AddressResolver {
} }
/** /**
* Sets the nonce of the current ovmADDRESS. * Bumps the nonce of the current ovmADDRESS by one.
* @param _nonce New nonce for the current contract.
*/ */
function ovmSETNONCE( function ovmINCREMENTNONCE()
uint256 _nonce
)
override override
public public
notStatic notStatic
{ {
_setAccountNonce(ovmADDRESS(), _nonce); address account = ovmADDRESS();
uint256 nonce = _getAccountNonce(account);
// Prevent overflow.
if (nonce + 1 > nonce) {
_setAccountNonce(account, nonce + 1);
}
} }
/** /**
......
...@@ -118,7 +118,7 @@ interface iOVM_ExecutionManager { ...@@ -118,7 +118,7 @@ interface iOVM_ExecutionManager {
******************************/ ******************************/
function ovmGETNONCE() external returns (uint256 _nonce); function ovmGETNONCE() external returns (uint256 _nonce);
function ovmSETNONCE(uint256 _nonce) external; function ovmINCREMENTNONCE() external;
function ovmCREATEEOA(bytes32 _messageHash, uint8 _v, bytes32 _r, bytes32 _s) external; function ovmCREATEEOA(bytes32 _messageHash, uint8 _v, bytes32 _r, bytes32 _s) external;
......
...@@ -208,18 +208,14 @@ library Lib_SafeExecutionManagerWrapper { ...@@ -208,18 +208,14 @@ library Lib_SafeExecutionManagerWrapper {
} }
/** /**
* Performs a safe ovmSETNONCE call. * Performs a safe ovmINCREMENTNONCE call.
* @param _nonce New account nonce.
*/ */
function safeSETNONCE( function safeINCREMENTNONCE()
uint256 _nonce
)
internal internal
{ {
_safeExecutionManagerInteraction( _safeExecutionManagerInteraction(
abi.encodeWithSignature( abi.encodeWithSignature(
"ovmSETNONCE(uint256)", "ovmINCREMENTNONCE()"
_nonce
) )
); );
} }
......
...@@ -65,7 +65,7 @@ contract mockOVM_ECDSAContractAccount is iOVM_ECDSAContractAccount { ...@@ -65,7 +65,7 @@ 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(decodedTx.nonce + 1); Lib_SafeExecutionManagerWrapper.safeINCREMENTNONCE();
return Lib_SafeExecutionManagerWrapper.safeCALL( return Lib_SafeExecutionManagerWrapper.safeCALL(
decodedTx.gasLimit, decodedTx.gasLimit,
......
...@@ -109,10 +109,6 @@ describe('OVM_ECDSAContractAccount', () => { ...@@ -109,10 +109,6 @@ describe('OVM_ECDSAContractAccount', () => {
expect(ovmCALL._gasLimit).to.equal(DEFAULT_EIP155_TX.gasLimit) expect(ovmCALL._gasLimit).to.equal(DEFAULT_EIP155_TX.gasLimit)
expect(ovmCALL._address).to.equal(DEFAULT_EIP155_TX.to) expect(ovmCALL._address).to.equal(DEFAULT_EIP155_TX.to)
expect(ovmCALL._calldata).to.equal(DEFAULT_EIP155_TX.data) expect(ovmCALL._calldata).to.equal(DEFAULT_EIP155_TX.data)
const ovmSETNONCE: any =
Mock__OVM_ExecutionManager.smocked.ovmSETNONCE.calls[0]
expect(ovmSETNONCE._nonce).to.equal(DEFAULT_EIP155_TX.nonce + 1)
}) })
it(`should successfully execute an ETHSignedTransaction`, async () => { it(`should successfully execute an ETHSignedTransaction`, async () => {
...@@ -137,10 +133,6 @@ describe('OVM_ECDSAContractAccount', () => { ...@@ -137,10 +133,6 @@ describe('OVM_ECDSAContractAccount', () => {
expect(ovmCALL._gasLimit).to.equal(DEFAULT_EIP155_TX.gasLimit) expect(ovmCALL._gasLimit).to.equal(DEFAULT_EIP155_TX.gasLimit)
expect(ovmCALL._address).to.equal(DEFAULT_EIP155_TX.to) expect(ovmCALL._address).to.equal(DEFAULT_EIP155_TX.to)
expect(ovmCALL._calldata).to.equal(DEFAULT_EIP155_TX.data) expect(ovmCALL._calldata).to.equal(DEFAULT_EIP155_TX.data)
const ovmSETNONCE: any =
Mock__OVM_ExecutionManager.smocked.ovmSETNONCE.calls[0]
expect(ovmSETNONCE._nonce).to.equal(DEFAULT_EIP155_TX.nonce + 1)
}) })
it(`should ovmCREATE if EIP155Transaction.to is zero address`, async () => { it(`should ovmCREATE if EIP155Transaction.to is zero address`, async () => {
......
...@@ -98,7 +98,7 @@ const test_ovmCREATEEOA: TestDefinition = { ...@@ -98,7 +98,7 @@ const test_ovmCREATEEOA: TestDefinition = {
], ],
}, },
{ {
name: 'ovmCALL(ADDRESS_1) => ovmSETNONCE(3) => ovmGETNONCE', name: 'ovmCALL(ADDRESS_1) => ovmINCREMENTNONCEx3 => ovmGETNONCE',
steps: [ steps: [
{ {
functionName: 'ovmCALL', functionName: 'ovmCALL',
...@@ -107,10 +107,15 @@ const test_ovmCREATEEOA: TestDefinition = { ...@@ -107,10 +107,15 @@ const test_ovmCREATEEOA: TestDefinition = {
target: '$DUMMY_OVM_ADDRESS_1', target: '$DUMMY_OVM_ADDRESS_1',
subSteps: [ subSteps: [
{ {
functionName: 'ovmSETNONCE', functionName: 'ovmINCREMENTNONCE',
functionParams: { expectedReturnStatus: true,
_nonce: '0x03', },
{
functionName: 'ovmINCREMENTNONCE',
expectedReturnStatus: true,
}, },
{
functionName: 'ovmINCREMENTNONCE',
expectedReturnStatus: true, expectedReturnStatus: true,
}, },
{ {
......
...@@ -28,7 +28,6 @@ import { ...@@ -28,7 +28,6 @@ import {
isTestStep_EXTCODEHASH, isTestStep_EXTCODEHASH,
isTestStep_EXTCODECOPY, isTestStep_EXTCODECOPY,
isTestStep_REVERT, isTestStep_REVERT,
isTestStep_SETNONCE,
} from './test.types' } from './test.types'
import { encodeRevertData, REVERT_FLAGS } from '../codec' import { encodeRevertData, REVERT_FLAGS } from '../codec'
import { import {
...@@ -402,7 +401,6 @@ export class ExecutionManagerTestRunner { ...@@ -402,7 +401,6 @@ export class ExecutionManagerTestRunner {
if ( if (
isTestStep_SSTORE(step) || isTestStep_SSTORE(step) ||
isTestStep_SLOAD(step) || isTestStep_SLOAD(step) ||
isTestStep_SETNONCE(step) ||
isTestStep_EXTCODESIZE(step) || isTestStep_EXTCODESIZE(step) ||
isTestStep_EXTCODEHASH(step) || isTestStep_EXTCODEHASH(step) ||
isTestStep_EXTCODECOPY(step) || isTestStep_EXTCODECOPY(step) ||
......
...@@ -87,11 +87,8 @@ interface TestStep_SLOAD { ...@@ -87,11 +87,8 @@ interface TestStep_SLOAD {
expectedReturnValue: string | RevertFlagError expectedReturnValue: string | RevertFlagError
} }
interface TestStep_SETNONCE { interface TestStep_INCREMENTNONCE {
functionName: 'ovmSETNONCE' functionName: 'ovmINCREMENTNONCE'
functionParams: {
_nonce: string
}
expectedReturnStatus: boolean expectedReturnStatus: boolean
expectedReturnValue?: RevertFlagError expectedReturnValue?: RevertFlagError
} }
...@@ -176,7 +173,7 @@ export type TestStep = ...@@ -176,7 +173,7 @@ export type TestStep =
| TestStep_Context | TestStep_Context
| TestStep_SSTORE | TestStep_SSTORE
| TestStep_SLOAD | TestStep_SLOAD
| TestStep_SETNONCE | TestStep_INCREMENTNONCE
| TestStep_CALL | TestStep_CALL
| TestStep_CREATE | TestStep_CREATE
| TestStep_CREATE2 | TestStep_CREATE2
...@@ -234,10 +231,10 @@ export const isTestStep_SLOAD = (step: TestStep): step is TestStep_SLOAD => { ...@@ -234,10 +231,10 @@ export const isTestStep_SLOAD = (step: TestStep): step is TestStep_SLOAD => {
return step.functionName === 'ovmSLOAD' return step.functionName === 'ovmSLOAD'
} }
export const isTestStep_SETNONCE = ( export const isTestStep_INCREMENTNONCE = (
step: TestStep step: TestStep
): step is TestStep_SETNONCE => { ): step is TestStep_INCREMENTNONCE => {
return step.functionName === 'ovmSETNONCE' return step.functionName === 'ovmINCREMENTNONCE'
} }
export const isTestStep_EXTCODESIZE = ( export const isTestStep_EXTCODESIZE = (
......
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