Commit 1caa5de7 authored by Maurelian's avatar Maurelian Committed by Kelvin Fichter

refactor(contracts): remove unused code

refactor(contracts): remove unused imports

test(contracts): reduce target costs to account for gas saving
parent 2775f256
...@@ -4,15 +4,11 @@ pragma solidity ^0.8.8; ...@@ -4,15 +4,11 @@ pragma solidity ^0.8.8;
/* Library Imports */ /* Library Imports */
import { Lib_OVMCodec } from "../../libraries/codec/Lib_OVMCodec.sol"; import { Lib_OVMCodec } from "../../libraries/codec/Lib_OVMCodec.sol";
import { Lib_AddressResolver } from "../../libraries/resolver/Lib_AddressResolver.sol"; import { Lib_AddressResolver } from "../../libraries/resolver/Lib_AddressResolver.sol";
import { Lib_MerkleTree } from "../../libraries/utils/Lib_MerkleTree.sol";
/* Interface Imports */ /* Interface Imports */
import { ICanonicalTransactionChain } from "./ICanonicalTransactionChain.sol"; import { ICanonicalTransactionChain } from "./ICanonicalTransactionChain.sol";
import { IChainStorageContainer } from "./IChainStorageContainer.sol"; import { IChainStorageContainer } from "./IChainStorageContainer.sol";
/* External Imports */
import { Math } from "@openzeppelin/contracts/utils/math/Math.sol";
/** /**
* @title CanonicalTransactionChain * @title CanonicalTransactionChain
* @dev The Canonical Transaction Chain (CTC) contract is an append-only log of transactions * @dev The Canonical Transaction Chain (CTC) contract is an append-only log of transactions
...@@ -426,44 +422,6 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes ...@@ -426,44 +422,6 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
); );
} }
/**
* Verifies whether a transaction is included in the chain.
* @param _transaction Transaction to verify.
* @param _txChainElement Transaction chain element corresponding to the transaction.
* @param _batchHeader Header of the batch the transaction was included in.
* @param _inclusionProof Inclusion proof for the provided transaction chain element.
* @return True if the transaction exists in the CTC, false if not.
*/
function verifyTransaction(
Lib_OVMCodec.Transaction memory _transaction,
Lib_OVMCodec.TransactionChainElement memory _txChainElement,
Lib_OVMCodec.ChainBatchHeader memory _batchHeader,
Lib_OVMCodec.ChainInclusionProof memory _inclusionProof
)
external
view
returns (
bool
)
{
if (_txChainElement.isSequenced == true) {
return _verifySequencerTransaction(
_transaction,
_txChainElement,
_batchHeader,
_inclusionProof
);
} else {
return _verifyQueueTransaction(
_transaction,
_txChainElement.queueIndex,
_batchHeader,
_inclusionProof
);
}
}
/********************** /**********************
* Internal Functions * * Internal Functions *
**********************/ **********************/
...@@ -575,31 +533,6 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes ...@@ -575,31 +533,6 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
return extraData; return extraData;
} }
/**
* Retrieves the hash of a queue element.
* @param _index Index of the queue element to retrieve a hash for.
* @return Hash of the queue element.
*/
function _getQueueLeafHash(
uint256 _index
)
internal
pure
returns (
bytes32
)
{
return _hashTransactionChainElement(
Lib_OVMCodec.TransactionChainElement({
isSequenced: false,
queueIndex: _index,
timestamp: 0,
blockNumber: 0,
txData: hex""
})
);
}
/** /**
* Gets the queue element at a particular index. * Gets the queue element at a particular index.
* @param _index Index of the queue element to access. * @param _index Index of the queue element to access.
...@@ -819,138 +752,4 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes ...@@ -819,138 +752,4 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
) )
); );
} }
/**
* Verifies a sequencer transaction, returning true if it was indeed included in the CTC
* @param _transaction The transaction we are verifying inclusion of.
* @param _txChainElement The chain element that the transaction is claimed to be a part of.
* @param _batchHeader Header of the batch the transaction was included in.
* @param _inclusionProof An inclusion proof into the CTC at a particular index.
* @return True if the transaction was included in the specified location, else false.
*/
function _verifySequencerTransaction(
Lib_OVMCodec.Transaction memory _transaction,
Lib_OVMCodec.TransactionChainElement memory _txChainElement,
Lib_OVMCodec.ChainBatchHeader memory _batchHeader,
Lib_OVMCodec.ChainInclusionProof memory _inclusionProof
)
internal
view
returns (
bool
)
{
bytes32 leafHash = _getSequencerLeafHash(_txChainElement);
require(
_verifyElement(
leafHash,
_batchHeader,
_inclusionProof
),
"Invalid Sequencer transaction inclusion proof."
);
require(
_transaction.blockNumber == _txChainElement.blockNumber
&& _transaction.timestamp == _txChainElement.timestamp
&& _transaction.entrypoint == address(0)
&& _transaction.gasLimit == maxTransactionGasLimit
&& _transaction.l1TxOrigin == address(0)
&& _transaction.l1QueueOrigin == Lib_OVMCodec.QueueOrigin.SEQUENCER_QUEUE
&& keccak256(_transaction.data) == keccak256(_txChainElement.txData),
"Invalid Sequencer transaction."
);
return true;
}
/**
* Verifies a queue transaction, returning true if it was indeed included in the CTC
* @param _transaction The transaction we are verifying inclusion of.
* @param _queueIndex The queueIndex of the queued transaction.
* @param _batchHeader Header of the batch the transaction was included in.
* @param _inclusionProof An inclusion proof into the CTC at a particular index (should point to
* queue tx).
* @return True if the transaction was included in the specified location, else false.
*/
function _verifyQueueTransaction(
Lib_OVMCodec.Transaction memory _transaction,
uint256 _queueIndex,
Lib_OVMCodec.ChainBatchHeader memory _batchHeader,
Lib_OVMCodec.ChainInclusionProof memory _inclusionProof
)
internal
view
returns (
bool
)
{
bytes32 leafHash = _getQueueLeafHash(_queueIndex);
require(
_verifyElement(
leafHash,
_batchHeader,
_inclusionProof
),
"Invalid Queue transaction inclusion proof."
);
bytes32 transactionHash = keccak256(
abi.encode(
_transaction.l1TxOrigin,
_transaction.entrypoint,
_transaction.gasLimit,
_transaction.data
)
);
Lib_OVMCodec.QueueElement memory el = getQueueElement(_queueIndex);
require(
el.transactionHash == transactionHash
&& el.timestamp == _transaction.timestamp
&& el.blockNumber == _transaction.blockNumber,
"Invalid Queue transaction."
);
return true;
}
/**
* Verifies a batch inclusion proof.
* @param _element Hash of the element to verify a proof for.
* @param _batchHeader Header of the batch in which the element was included.
* @param _proof Merkle inclusion proof for the element.
*/
function _verifyElement(
bytes32 _element,
Lib_OVMCodec.ChainBatchHeader memory _batchHeader,
Lib_OVMCodec.ChainInclusionProof memory _proof
)
internal
view
returns (
bool
)
{
require(
Lib_OVMCodec.hashBatchHeader(_batchHeader) ==
batches().get(uint32(_batchHeader.batchIndex)),
"Invalid batch header."
);
require(
Lib_MerkleTree.verify(
_batchHeader.batchRoot,
_element,
_proof.index,
_proof.siblings,
_batchHeader.batchSize
),
"Invalid inclusion proof."
);
return true;
}
} }
...@@ -206,24 +206,4 @@ interface ICanonicalTransactionChain { ...@@ -206,24 +206,4 @@ interface ICanonicalTransactionChain {
// bytes[] _transactionDataFields // bytes[] _transactionDataFields
) )
external; external;
/**
* Verifies whether a transaction is included in the chain.
* @param _transaction Transaction to verify.
* @param _txChainElement Transaction chain element corresponding to the transaction.
* @param _batchHeader Header of the batch the transaction was included in.
* @param _inclusionProof Inclusion proof for the provided transaction chain element.
* @return True if the transaction exists in the CTC, false if not.
*/
function verifyTransaction(
Lib_OVMCodec.Transaction memory _transaction,
Lib_OVMCodec.TransactionChainElement memory _txChainElement,
Lib_OVMCodec.ChainBatchHeader memory _batchHeader,
Lib_OVMCodec.ChainInclusionProof memory _inclusionProof
)
external
view
returns (
bool
);
} }
...@@ -41,7 +41,7 @@ const appendSequencerBatch = async ( ...@@ -41,7 +41,7 @@ const appendSequencerBatch = async (
}) })
} }
const printGasSavings = (gasUsed:number, regenesis040Cost:number):void => { const printGasSavings = (gasUsed: number, regenesis040Cost: number): void => {
console.log(' - Gas used:', gasUsed) console.log(' - Gas used:', gasUsed)
console.log( console.log(
' - Absolute savings vs regenesis/0.4.0:', ' - Absolute savings vs regenesis/0.4.0:',
...@@ -49,8 +49,7 @@ const printGasSavings = (gasUsed:number, regenesis040Cost:number):void => { ...@@ -49,8 +49,7 @@ const printGasSavings = (gasUsed:number, regenesis040Cost:number):void => {
) )
console.log( console.log(
' - Relative savings vs regenesis/0.4.0:', ' - Relative savings vs regenesis/0.4.0:',
(((regenesis040Cost - gasUsed) / regenesis040Cost) * 100).toFixed(2) + (((regenesis040Cost - gasUsed) / regenesis040Cost) * 100).toFixed(2) + '%'
'%'
) )
} }
...@@ -171,7 +170,7 @@ describe('[GAS BENCHMARK] CanonicalTransactionChain', () => { ...@@ -171,7 +170,7 @@ describe('[GAS BENCHMARK] CanonicalTransactionChain', () => {
'Non-calldata overhead gas cost per transaction:', 'Non-calldata overhead gas cost per transaction:',
(gasUsed - fixedCalldataCost) / numTxs (gasUsed - fixedCalldataCost) / numTxs
) )
expectApprox(gasUsed, 1_767_570, { expectApprox(gasUsed, 1_422_181, {
upperPercentDeviation: 0, upperPercentDeviation: 0,
// Assert a lower bound of 1% reduction on gas cost. If your tests are breaking because your // Assert a lower bound of 1% reduction on gas cost. If your tests are breaking because your
// contracts are too efficient, consider updating the target value! // contracts are too efficient, consider updating the target value!
...@@ -212,14 +211,14 @@ describe('[GAS BENCHMARK] CanonicalTransactionChain', () => { ...@@ -212,14 +211,14 @@ describe('[GAS BENCHMARK] CanonicalTransactionChain', () => {
const gasUsed = receipt.gasUsed.toNumber() const gasUsed = receipt.gasUsed.toNumber()
console.log('Benchmark complete.') console.log('Benchmark complete.')
printGasSavings(gasUsed, 1_787_052) printGasSavings(gasUsed, 1_632_687)
console.log('Fixed calldata cost:', fixedCalldataCost) console.log('Fixed calldata cost:', fixedCalldataCost)
console.log( console.log(
'Non-calldata overhead gas cost per transaction:', 'Non-calldata overhead gas cost per transaction:',
(gasUsed - fixedCalldataCost) / numTxs (gasUsed - fixedCalldataCost) / numTxs
) )
expectApprox(gasUsed, 1_950_378, { expectApprox(gasUsed, 1_632_687, {
upperPercentDeviation: 0, upperPercentDeviation: 0,
// Assert a lower bound of 1% reduction on gas cost. If your tests are breaking because your // Assert a lower bound of 1% reduction on gas cost. If your tests are breaking because your
// contracts are too efficient, consider updating the target value! // contracts are too efficient, consider updating the target value!
......
...@@ -29,28 +29,6 @@ import { predeploys } from '../../../../src' ...@@ -29,28 +29,6 @@ import { predeploys } from '../../../../src'
const ELEMENT_TEST_SIZES = [1, 2, 4, 8, 16] const ELEMENT_TEST_SIZES = [1, 2, 4, 8, 16]
const MAX_GAS_LIMIT = 8_000_000 const MAX_GAS_LIMIT = 8_000_000
const getQueueLeafHash = (index: number): string => {
return keccak256(
ethers.utils.defaultAbiCoder.encode(
['bool', 'uint256', 'uint256', 'uint256', 'bytes'],
[false, index, 0, 0, '0x']
)
)
}
const getSequencerLeafHash = (
timestamp: number,
blockNumber: number,
data: string
): string => {
return keccak256(
'0x01' +
remove0x(BigNumber.from(timestamp).toHexString()).padStart(64, '0') +
remove0x(BigNumber.from(blockNumber).toHexString()).padStart(64, '0') +
remove0x(data)
)
}
const getTransactionHash = ( const getTransactionHash = (
sender: string, sender: string,
target: string, target: string,
...@@ -417,166 +395,6 @@ describe('CanonicalTransactionChain', () => { ...@@ -417,166 +395,6 @@ describe('CanonicalTransactionChain', () => {
}) })
}) })
describe('verifyTransaction', () => {
it('should successfully verify against a valid queue transaction appended by the sequencer', async () => {
const entrypoint = NON_ZERO_ADDRESS
const gasLimit = 500_000
const data = '0x' + '12'.repeat(1234)
const timestamp = (await getEthTime(ethers.provider)) + 100
await setEthTime(ethers.provider, timestamp)
await CanonicalTransactionChain.enqueue(entrypoint, gasLimit, data)
const blockNumber = await ethers.provider.getBlockNumber()
await appendSequencerBatch(CanonicalTransactionChain.connect(sequencer), {
shouldStartAtElement: 0,
totalElementsToAppend: 1,
contexts: [
{
numSequencedTransactions: 0,
numSubsequentQueueTransactions: 1,
timestamp,
blockNumber,
},
],
transactions: [],
})
expect(
await CanonicalTransactionChain.verifyTransaction(
{
timestamp,
blockNumber,
l1QueueOrigin: 1,
l1TxOrigin: await CanonicalTransactionChain.signer.getAddress(),
entrypoint,
gasLimit,
data,
},
{
isSequenced: false,
queueIndex: 0,
timestamp: 0,
blockNumber: 0,
txData: '0x',
},
{
batchIndex: 0,
batchRoot: getQueueLeafHash(0),
batchSize: 1,
prevTotalElements: 0,
extraData: '0x',
},
{
index: 0,
siblings: [],
}
)
).to.equal(true)
})
it.skip('should successfully verify against a valid queue transaction appended by force', async () => {
const entrypoint = NON_ZERO_ADDRESS
const gasLimit = 500_000
const data = '0x' + '12'.repeat(1234)
const timestamp = (await getEthTime(ethers.provider)) + 100
await setEthTime(ethers.provider, timestamp)
await CanonicalTransactionChain.enqueue(entrypoint, gasLimit, data)
const blockNumber = await ethers.provider.getBlockNumber()
await CanonicalTransactionChain.appendQueueBatch(1)
expect(
await CanonicalTransactionChain.verifyTransaction(
{
timestamp,
blockNumber,
l1QueueOrigin: 1,
l1TxOrigin: await CanonicalTransactionChain.signer.getAddress(),
entrypoint,
gasLimit,
data,
},
{
isSequenced: false,
queueIndex: 0,
timestamp: 0,
blockNumber: 0,
txData: '0x',
},
{
batchIndex: 0,
batchRoot: getQueueLeafHash(0),
batchSize: 1,
prevTotalElements: 0,
extraData: '0x',
},
{
index: 0,
siblings: [],
}
)
).to.equal(true)
})
it('should successfully verify against a valid sequencer transaction', async () => {
const entrypoint = ethers.constants.AddressZero
const gasLimit = MAX_GAS_LIMIT
const data = '0x' + '12'.repeat(1234)
const timestamp = (await getEthTime(ethers.provider)) - 10
const blockNumber = (await ethers.provider.getBlockNumber()) - 1
await appendSequencerBatch(CanonicalTransactionChain.connect(sequencer), {
shouldStartAtElement: 0,
totalElementsToAppend: 1,
contexts: [
{
numSequencedTransactions: 1,
numSubsequentQueueTransactions: 0,
timestamp,
blockNumber,
},
],
transactions: [data],
})
expect(
await CanonicalTransactionChain.verifyTransaction(
{
timestamp,
blockNumber,
l1QueueOrigin: 0,
l1TxOrigin: constants.AddressZero,
entrypoint,
gasLimit,
data,
},
{
isSequenced: true,
queueIndex: 0,
timestamp,
blockNumber,
txData: data,
},
{
batchIndex: 0,
batchRoot: getSequencerLeafHash(timestamp, blockNumber, data),
batchSize: 1,
prevTotalElements: 0,
extraData: '0x',
},
{
index: 0,
siblings: [],
}
)
).to.equal(true)
})
})
describe('appendSequencerBatch', () => { describe('appendSequencerBatch', () => {
beforeEach(() => { beforeEach(() => {
CanonicalTransactionChain = CanonicalTransactionChain.connect(sequencer) CanonicalTransactionChain = CanonicalTransactionChain.connect(sequencer)
......
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