Skip to content

Commit

Permalink
refactor(contracts): remove unused code from CTC
Browse files Browse the repository at this point in the history
  • Loading branch information
maurelian authored and smartcontracts committed Oct 1, 2021
1 parent dc24b38 commit 1b968b1
Show file tree
Hide file tree
Showing 10 changed files with 9 additions and 196 deletions.
5 changes: 5 additions & 0 deletions .changeset/few-students-pretend.md
@@ -0,0 +1,5 @@
---
'@eth-optimism/contracts': minor
---

Remove unused code from CTC
5 changes: 0 additions & 5 deletions packages/contracts/bin/deploy.ts
Expand Up @@ -31,10 +31,6 @@ const parseEnv = () => {

return {
l1BlockTimeSeconds: ensure('BLOCK_TIME_SECONDS', 'number'),
ctcForceInclusionPeriodSeconds: ensure(
'FORCE_INCLUSION_PERIOD_SECONDS',
'number'
),
ctcMaxTransactionGasLimit: ensure('MAX_TRANSACTION_GAS_LIMIT', 'number'),
ctcL2GasDiscountDivisor: ensure('L2_GAS_DISCOUNT_DIVISOR', 'number'),
ctcEnqueueGasCost: ensure('ENQUEUE_GAS_COST', 'number'),
Expand All @@ -51,7 +47,6 @@ const main = async () => {

await hre.run('deploy', {
l1BlockTimeSeconds: config.l1BlockTimeSeconds,
ctcForceInclusionPeriodSeconds: config.ctcForceInclusionPeriodSeconds,
ctcMaxTransactionGasLimit: config.ctcMaxTransactionGasLimit,
ctcL2GasDiscountDivisor: config.ctcL2GasDiscountDivisor,
ctcEnqueueGasCost: config.ctcEnqueueGasCost,
Expand Down
Expand Up @@ -20,8 +20,6 @@ import { Math } from "@openzeppelin/contracts/utils/math/Math.sol";
* writing them to the 'CTC:batches' instance of the Chain Storage Container.
* The CTC also allows any account to 'enqueue' an L2 transaction, which will require that the
* Sequencer will eventually append it to the rollup state.
* If the Sequencer does not include an enqueued transaction within the 'force inclusion period',
* then any account may force it to be included by calling appendQueueBatch().
*
* Runtime target: EVM
*/
Expand Down Expand Up @@ -50,8 +48,6 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
* Variables *
*************/

uint256 public forceInclusionPeriodSeconds;
uint256 public forceInclusionPeriodBlocks;
uint256 public maxTransactionGasLimit;


Expand All @@ -61,16 +57,12 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes

constructor(
address _libAddressManager,
uint256 _forceInclusionPeriodSeconds,
uint256 _forceInclusionPeriodBlocks,
uint256 _maxTransactionGasLimit,
uint256 _l2GasDiscountDivisor,
uint256 _enqueueGasCost
)
Lib_AddressResolver(_libAddressManager)
{
forceInclusionPeriodSeconds = _forceInclusionPeriodSeconds;
forceInclusionPeriodBlocks = _forceInclusionPeriodBlocks;
maxTransactionGasLimit = _maxTransactionGasLimit;
L2_GAS_DISCOUNT_DIVISOR = _l2GasDiscountDivisor;
ENQUEUE_GAS_COST = _enqueueGasCost;
Expand Down Expand Up @@ -250,7 +242,7 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
uint256 _gasLimit,
bytes memory _data
)
public
external
{
require(
_data.length <= MAX_ROLLUP_TX_SIZE,
Expand Down Expand Up @@ -325,59 +317,6 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
);
}

/**
* Appends a given number of queued transactions as a single batch.
* param _numQueuedTransactions Number of transactions to append.
*/
function appendQueueBatch(
uint256 // _numQueuedTransactions
)
public
pure
{
// TEMPORARY: Disable `appendQueueBatch` for minnet
revert("appendQueueBatch is currently disabled.");

// solhint-disable max-line-length
// _numQueuedTransactions = Math.min(_numQueuedTransactions, getNumPendingQueueElements());
// require(
// _numQueuedTransactions > 0,
// "Must append more than zero transactions."
// );

// bytes32[] memory leaves = new bytes32[](_numQueuedTransactions);
// uint40 nextQueueIndex = getNextQueueIndex();

// for (uint256 i = 0; i < _numQueuedTransactions; i++) {
// if (msg.sender != resolve("OVM_Sequencer")) {
// Lib_OVMCodec.QueueElement memory el = getQueueElement(nextQueueIndex);
// require(
// el.timestamp + forceInclusionPeriodSeconds < block.timestamp,
// "Queue transactions cannot be submitted during the sequencer inclusion period."
// );
// }
// leaves[i] = _getQueueLeafHash(nextQueueIndex);
// nextQueueIndex++;
// }

// Lib_OVMCodec.QueueElement memory lastElement = getQueueElement(nextQueueIndex - 1);

// _appendBatch(
// Lib_MerkleTree.getMerkleRoot(leaves),
// _numQueuedTransactions,
// _numQueuedTransactions,
// lastElement.timestamp,
// lastElement.blockNumber
// );

// emit QueueBatchAppended(
// nextQueueIndex - _numQueuedTransactions,
// _numQueuedTransactions,
// getTotalElements()
// );
// solhint-enable max-line-length
}

/**
* Allows the sequencer to append a batch of transactions.
* @dev This function uses a custom encoding scheme for efficiency reasons.
Expand All @@ -387,7 +326,7 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
* .param _transactionDataFields Array of raw transaction data.
*/
function appendSequencerBatch()
public
external
{
uint40 shouldStartAtElement;
uint24 totalElementsToAppend;
Expand Down Expand Up @@ -540,7 +479,7 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
Lib_OVMCodec.ChainBatchHeader memory _batchHeader,
Lib_OVMCodec.ChainInclusionProof memory _inclusionProof
)
public
external
view
returns (
bool
Expand Down
Expand Up @@ -191,15 +191,6 @@ interface ICanonicalTransactionChain {
)
external;

/**
* Appends a given number of queued transactions as a single batch.
* @param _numQueuedTransactions Number of transactions to append.
*/
function appendQueueBatch(
uint256 _numQueuedTransactions
)
external;

/**
* Allows the sequencer to append a batch of transactions.
* @dev This function uses a custom encoding scheme for efficiency reasons.
Expand Down
Expand Up @@ -18,8 +18,6 @@ const deployFn: DeployFunction = async (hre) => {
name: 'CanonicalTransactionChain',
args: [
Lib_AddressManager.address,
(hre as any).deployConfig.ctcForceInclusionPeriodSeconds,
(hre as any).deployConfig.ctcForceInclusionPeriodBlocks,
(hre as any).deployConfig.ctcMaxTransactionGasLimit,
(hre as any).deployConfig.ctcL2GasDiscountDivisor,
(hre as any).deployConfig.ctcEnqueueGasCost,
Expand Down
11 changes: 0 additions & 11 deletions packages/contracts/tasks/deploy.ts
Expand Up @@ -4,7 +4,6 @@ import { task } from 'hardhat/config'
import * as types from 'hardhat/internal/core/params/argumentTypes'

const DEFAULT_L1_BLOCK_TIME_SECONDS = 15
const DEFAULT_CTC_FORCE_INCLUSION_PERIOD_SECONDS = 60 * 60 * 24 * 30 // 30 days
const DEFAULT_CTC_MAX_TRANSACTION_GAS_LIMIT = 11_000_000
const DEFAULT_CTC_L2_GAS_DISCOUNT_DIVISOR = 32
const DEFAULT_CTC_ENQUEUE_GAS_COST = 60_000
Expand All @@ -18,12 +17,6 @@ task('deploy')
DEFAULT_L1_BLOCK_TIME_SECONDS,
types.int
)
.addOptionalParam(
'ctcForceInclusionPeriodSeconds',
'Number of seconds that the sequencer has to include transactions before the L1 queue.',
DEFAULT_CTC_FORCE_INCLUSION_PERIOD_SECONDS,
types.int
)
.addOptionalParam(
'ctcMaxTransactionGasLimit',
'Max gas limit for L1 queue transactions.',
Expand Down Expand Up @@ -91,10 +84,6 @@ task('deploy')
validateAddressArg('ovmProposerAddress')
validateAddressArg('ovmAddressManagerOwner')

args.ctcForceInclusionPeriodBlocks = Math.floor(
args.ctcForceInclusionPeriodSeconds / args.l1BlockTimeSeconds
)

hre.deployConfig = args
return runSuper(args)
})
Expand Up @@ -14,8 +14,6 @@ import {
NON_ZERO_ADDRESS,
DUMMY_BATCH_HEADERS,
DUMMY_BATCH_PROOFS,
FORCE_INCLUSION_PERIOD_SECONDS,
FORCE_INCLUSION_PERIOD_BLOCKS,
L2_GAS_DISCOUNT_DIVISOR,
ENQUEUE_GAS_COST,
TrieTestGenerator,
Expand Down Expand Up @@ -100,8 +98,6 @@ describe('L1CrossDomainMessenger', () => {
)
CanonicalTransactionChain = await Factory__CanonicalTransactionChain.deploy(
AddressManager.address,
FORCE_INCLUSION_PERIOD_SECONDS,
FORCE_INCLUSION_PERIOD_BLOCKS,
MAX_GAS_LIMIT,
L2_GAS_DISCOUNT_DIVISOR,
ENQUEUE_GAS_COST
Expand Down
Expand Up @@ -6,8 +6,6 @@ import { smoddit } from '@eth-optimism/smock'
/* Internal Imports */
import {
makeAddressManager,
FORCE_INCLUSION_PERIOD_SECONDS,
FORCE_INCLUSION_PERIOD_BLOCKS,
L2_GAS_DISCOUNT_DIVISOR,
ENQUEUE_GAS_COST,
NON_ZERO_ADDRESS,
Expand Down Expand Up @@ -46,8 +44,6 @@ describe('[GAS BENCHMARK] Depositing via the standard bridge', () => {
await ethers.getContractFactory('CanonicalTransactionChain')
).deploy(
AddressManager.address,
FORCE_INCLUSION_PERIOD_SECONDS,
FORCE_INCLUSION_PERIOD_BLOCKS,
MAX_GAS_LIMIT,
L2_GAS_DISCOUNT_DIVISOR,
ENQUEUE_GAS_COST
Expand Down
Expand Up @@ -81,13 +81,8 @@ describe('[GAS BENCHMARK] CanonicalTransactionChain', () => {

let CanonicalTransactionChain: Contract
beforeEach(async () => {
// Use a larger FIP for blocks so that we can send a large number of
// enqueue() transactions without having to manipulate the block number.
const forceInclusionPeriodBlocks = 101
CanonicalTransactionChain = await Factory__CanonicalTransactionChain.deploy(
AddressManager.address,
FORCE_INCLUSION_PERIOD_SECONDS,
forceInclusionPeriodBlocks,
MAX_GAS_LIMIT,
L2_GAS_DISCOUNT_DIVISOR,
ENQUEUE_GAS_COST
Expand Down Expand Up @@ -269,7 +264,7 @@ describe('[GAS BENCHMARK] CanonicalTransactionChain', () => {
'Non-calldata overhead gas cost per transaction:',
(gasUsed - fixedCalldataCost) / numTxs
)
expectApprox(gasUsed, 1_293_111, { upperPercentDeviation: 0 })
expectApprox(gasUsed, 1_293_611, { upperPercentDeviation: 0 })
}).timeout(10_000_000)
})

Expand Down
Expand Up @@ -17,17 +17,12 @@ import _ from 'lodash'
import {
makeAddressManager,
setProxyTarget,
FORCE_INCLUSION_PERIOD_SECONDS,
FORCE_INCLUSION_PERIOD_BLOCKS,
L2_GAS_DISCOUNT_DIVISOR,
ENQUEUE_GAS_COST,
setEthTime,
NON_ZERO_ADDRESS,
getEthTime,
getNextBlockNumber,
increaseEthTime,
getBlockTime,
mineBlock,
} from '../../../helpers'
import { predeploys } from '../../../../src'

Expand Down Expand Up @@ -132,8 +127,6 @@ describe('CanonicalTransactionChain', () => {
beforeEach(async () => {
CanonicalTransactionChain = await Factory__CanonicalTransactionChain.deploy(
AddressManager.address,
FORCE_INCLUSION_PERIOD_SECONDS,
FORCE_INCLUSION_PERIOD_BLOCKS,
MAX_GAS_LIMIT,
L2_GAS_DISCOUNT_DIVISOR,
ENQUEUE_GAS_COST
Expand Down Expand Up @@ -424,89 +417,6 @@ describe('CanonicalTransactionChain', () => {
})
})

describe('appendQueueBatch disabled', () => {
it('should revert', async () => {
await expect(
CanonicalTransactionChain.appendQueueBatch(0)
).to.be.revertedWith('appendQueueBatch is currently disabled.')
})
})

describe.skip('appendQueueBatch', () => {
it('should revert if trying to append zero transactions', async () => {
await expect(
CanonicalTransactionChain.appendQueueBatch(0)
).to.be.revertedWith('Must append more than zero transactions.')
})

it('should revert if the queue is empty', async () => {
await expect(
CanonicalTransactionChain.appendQueueBatch(1)
).to.be.revertedWith('Must append more than zero transactions.')
})

describe('when the queue is not empty', () => {
const target = NON_ZERO_ADDRESS
const gasLimit = 500_000
const data = '0x' + '12'.repeat(1234)

for (const size of ELEMENT_TEST_SIZES) {
describe(`when the queue has ${size} elements`, () => {
beforeEach(async () => {
for (let i = 0; i < size; i++) {
await CanonicalTransactionChain.enqueue(target, gasLimit, data)
}
})

describe('when the sequencer inclusion period has not passed', () => {
it('should revert if not called by the sequencer', async () => {
await expect(
CanonicalTransactionChain.connect(signer).appendQueueBatch(1)
).to.be.revertedWith(
'Queue transactions cannot be submitted during the sequencer inclusion period.'
)
})

it('should succeed if called by the sequencer', async () => {
await expect(
CanonicalTransactionChain.connect(sequencer).appendQueueBatch(1)
)
.to.emit(CanonicalTransactionChain, 'QueueBatchAppended')
.withArgs(0, 1, 1)
})
})

describe('when the sequencer inclusion period has passed', () => {
beforeEach(async () => {
await increaseEthTime(
ethers.provider,
FORCE_INCLUSION_PERIOD_SECONDS * 2
)
})

it('should be able to append a single element', async () => {
await expect(CanonicalTransactionChain.appendQueueBatch(1))
.to.emit(CanonicalTransactionChain, 'QueueBatchAppended')
.withArgs(0, 1, 1)
})

it(`should be able to append ${size} elements`, async () => {
await expect(CanonicalTransactionChain.appendQueueBatch(size))
.to.emit(CanonicalTransactionChain, 'QueueBatchAppended')
.withArgs(0, size, size)
})

it(`should be able to append ${size} elements even if attempting to append ${size} + 1 elements`, async () => {
await expect(CanonicalTransactionChain.appendQueueBatch(size + 1))
.to.emit(CanonicalTransactionChain, 'QueueBatchAppended')
.withArgs(0, size, size)
})
})
})
}
})
})

describe('verifyTransaction', () => {
it('should successfully verify against a valid queue transaction appended by the sequencer', async () => {
const entrypoint = NON_ZERO_ADDRESS
Expand Down Expand Up @@ -576,7 +486,6 @@ describe('CanonicalTransactionChain', () => {
await CanonicalTransactionChain.enqueue(entrypoint, gasLimit, data)

const blockNumber = await ethers.provider.getBlockNumber()
await increaseEthTime(ethers.provider, FORCE_INCLUSION_PERIOD_SECONDS * 2)

await CanonicalTransactionChain.appendQueueBatch(1)

Expand Down

0 comments on commit 1b968b1

Please sign in to comment.