Commit 5db50b3d authored by Maurelian's avatar Maurelian Committed by Kelvin Fichter

refactor(contracts): Remove the queue storage container.

chore: add changeset
parent d2eb8ae0
---
'@eth-optimism/contracts': minor
---
Replace the CTCs Queue storage container with a mapping
...@@ -48,6 +48,14 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes ...@@ -48,6 +48,14 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
uint256 public maxTransactionGasLimit; uint256 public maxTransactionGasLimit;
/***************
* Queue State *
***************/
uint40 private nextQueueIndex; // index of the first queue element not yet included
Lib_OVMCodec.QueueElement[] queueElements;
/*************** /***************
* Constructor * * Constructor *
***************/ ***************/
...@@ -143,7 +151,6 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes ...@@ -143,7 +151,6 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
uint40 uint40
) )
{ {
(,uint40 nextQueueIndex,,) = _getBatchExtraData();
return nextQueueIndex; return nextQueueIndex;
} }
...@@ -192,8 +199,7 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes ...@@ -192,8 +199,7 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
) )
{ {
return _getQueueElement( return _getQueueElement(
_index, _index
queue()
); );
} }
...@@ -223,9 +229,7 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes ...@@ -223,9 +229,7 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
uint40 uint40
) )
{ {
return _getQueueLength( return uint40(transactionHashes.length);
queue()
);
} }
/** /**
...@@ -301,21 +305,14 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes ...@@ -301,21 +305,14 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
) )
); );
bytes32 timestampAndBlockNumber; queueElements.push(
assembly { Lib_OVMCodec.QueueElement({
timestampAndBlockNumber := timestamp() transactionHash: transactionHash,
timestampAndBlockNumber := or(timestampAndBlockNumber, shl(40, number())) timestamp: uint40(block.timestamp),
} blockNumber: uint40(block.number)
})
IChainStorageContainer queueRef = queue(); );
uint256 queueIndex = queueElements.length - 1;
queueRef.push(transactionHash);
queueRef.push(timestampAndBlockNumber);
// The underlying queue data structure stores 2 elements
// per insertion, so to get the real queue length we need
// to divide by 2 and subtract 1.
uint256 queueIndex = queueRef.length() / 2 - 1;
emit TransactionEnqueued( emit TransactionEnqueued(
sender, sender,
_target, _target,
...@@ -365,18 +362,13 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes ...@@ -365,18 +362,13 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
"Not enough BatchContexts provided." "Not enough BatchContexts provided."
); );
// Take a reference to the queue and its length so we don't have to keep resolving it.
// Length isn't going to change during the course of execution, so it's fine to simply
// resolve this once at the start. Saves gas.
IChainStorageContainer queueRef = queue();
uint40 queueLength = _getQueueLength(queueRef);
// Counter for number of sequencer transactions appended so far. // Counter for number of sequencer transactions appended so far.
uint32 numSequencerTransactions = 0; uint32 numSequencerTransactions = 0;
// We will sequentially append leaves which are pointers to the queue. // Cache the nextQueueIndex.
// The initial queue index is what is currently in storage. // In order to be safe, nothing should read or write to the nextQueueIndex
uint40 nextQueueIndex = getNextQueueIndex(); // storage variable until it is updated with the value from nextQueueIndexCached.
uint40 nextQueueIndexCached = nextQueueIndex;
BatchContext memory curContext; BatchContext memory curContext;
for (uint32 i = 0; i < numContexts; i++) { for (uint32 i = 0; i < numContexts; i++) {
...@@ -389,11 +381,11 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes ...@@ -389,11 +381,11 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
numSequencerTransactions += uint32(curContext.numSequencedTransactions); numSequencerTransactions += uint32(curContext.numSequencedTransactions);
// Now process any subsequent queue transactions. // Now process any subsequent queue transactions.
nextQueueIndex += uint40(curContext.numSubsequentQueueTransactions); nextQueueIndexCached += uint40(curContext.numSubsequentQueueTransactions);
} }
require( require(
nextQueueIndex <= queueLength, nextQueueIndexCached <= queueElements.length,
"Attempted to append more elements than are available in the queue." "Attempted to append more elements than are available in the queue."
); );
...@@ -413,8 +405,7 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes ...@@ -413,8 +405,7 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
// least one queue element. We increment nextQueueIndex after processing each queue // least one queue element. We increment nextQueueIndex after processing each queue
// element, so the index of the last element we processed is nextQueueIndex - 1. // element, so the index of the last element we processed is nextQueueIndex - 1.
Lib_OVMCodec.QueueElement memory lastElement = _getQueueElement( Lib_OVMCodec.QueueElement memory lastElement = _getQueueElement(
nextQueueIndex - 1, nextQueueIndexCached - 1
queueRef
); );
blockTimestamp = lastElement.timestamp; blockTimestamp = lastElement.timestamp;
...@@ -431,10 +422,13 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes ...@@ -431,10 +422,13 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
); );
emit SequencerBatchAppended( emit SequencerBatchAppended(
nextQueueIndex - numQueuedTransactions, nextQueueIndexCached - numQueuedTransactions,
numQueuedTransactions, numQueuedTransactions,
getTotalElements() getTotalElements()
); );
// Update the nextQueueIndex storage variable.
nextQueueIndex = nextQueueIndexCached;
} }
/********************** /**********************
...@@ -554,8 +548,7 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes ...@@ -554,8 +548,7 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
* @return _element Queue element at the given index. * @return _element Queue element at the given index.
*/ */
function _getQueueElement( function _getQueueElement(
uint256 _index, uint256 _index
IChainStorageContainer _queueRef
) )
internal internal
view view
...@@ -563,46 +556,8 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes ...@@ -563,46 +556,8 @@ contract CanonicalTransactionChain is ICanonicalTransactionChain, Lib_AddressRes
Lib_OVMCodec.QueueElement memory _element Lib_OVMCodec.QueueElement memory _element
) )
{ {
// The underlying queue data structure stores 2 elements
// per insertion, so to get the actual desired queue index
// we need to multiply by 2.
uint40 trueIndex = uint40(_index * 2);
bytes32 transactionHash = _queueRef.get(trueIndex);
bytes32 timestampAndBlockNumber = _queueRef.get(trueIndex + 1);
uint40 elementTimestamp;
uint40 elementBlockNumber;
// solhint-disable max-line-length
assembly {
elementTimestamp := and(timestampAndBlockNumber, 0x000000000000000000000000000000000000000000000000000000FFFFFFFFFF)
elementBlockNumber := shr(40, and(timestampAndBlockNumber, 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF0000000000))
}
// solhint-enable max-line-length // solhint-enable max-line-length
return queueElements[_index];
return Lib_OVMCodec.QueueElement({
transactionHash: transactionHash,
timestamp: elementTimestamp,
blockNumber: elementBlockNumber
});
}
/**
* Retrieves the length of the queue.
* @return Length of the queue.
*/
function _getQueueLength(
IChainStorageContainer _queueRef
)
internal
view
returns (
uint40
)
{
// The underlying queue data structure stores 2 elements
// per insertion, so to get the real queue length we need
// to divide by 2.
return uint40(_queueRef.length() / 2);
} }
/** /**
......
...@@ -218,7 +218,7 @@ describe('[GAS BENCHMARK] CanonicalTransactionChain', () => { ...@@ -218,7 +218,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_632_687, { expectApprox(gasUsed, 1_617_381, {
absoluteUpperDeviation: 1000, absoluteUpperDeviation: 1000,
// 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!
...@@ -307,9 +307,9 @@ describe('[GAS BENCHMARK] CanonicalTransactionChain', () => { ...@@ -307,9 +307,9 @@ describe('[GAS BENCHMARK] CanonicalTransactionChain', () => {
const gasUsed = receipt.gasUsed.toNumber() const gasUsed = receipt.gasUsed.toNumber()
console.log('Benchmark complete.') console.log('Benchmark complete.')
console.log('Gas used:', gasUsed) printGasSavings(gasUsed, 237_065)
expectApprox(gasUsed, 218_203, { expectApprox(gasUsed, 206_880, {
absoluteUpperDeviation: 500, absoluteUpperDeviation: 500,
// 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!
...@@ -329,9 +329,9 @@ describe('[GAS BENCHMARK] CanonicalTransactionChain', () => { ...@@ -329,9 +329,9 @@ describe('[GAS BENCHMARK] CanonicalTransactionChain', () => {
const gasUsed = receipt.gasUsed.toNumber() const gasUsed = receipt.gasUsed.toNumber()
console.log('Benchmark complete.') console.log('Benchmark complete.')
console.log('Gas used:', gasUsed) printGasSavings(gasUsed, 196_457)
expectApprox(gasUsed, 157_822, { expectApprox(gasUsed, 146_499, {
absoluteUpperDeviation: 500, absoluteUpperDeviation: 500,
// 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!
......
...@@ -261,7 +261,9 @@ describe('CanonicalTransactionChain', () => { ...@@ -261,7 +261,9 @@ describe('CanonicalTransactionChain', () => {
it('should revert when accessing a non-existent element', async () => { it('should revert when accessing a non-existent element', async () => {
await expect( await expect(
CanonicalTransactionChain.getQueueElement(0) CanonicalTransactionChain.getQueueElement(0)
).to.be.revertedWith('Index out of bounds.') ).to.be.revertedWith(
'reverted with panic code 0x32 (Array accessed at an out-of-bounds or negative index)'
)
}) })
describe('when the requested element exists', () => { describe('when the requested element exists', () => {
......
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