Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EIP4844 smart contract updated #8712

Closed
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 98 additions & 13 deletions packages/contracts-bedrock/src/L2/GasPriceOracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,30 @@ contract GasPriceOracle is ISemver {
uint256 public constant DECIMALS = 6;

/// @notice Semantic version.
/// @custom:semver 1.1.0
string public constant version = "1.1.0";
/// @custom:semver 1.2.0
string public constant version = "1.2.0";

/// @notice Flag that indicates whether the network is in ecotone mode.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// @notice Flag that indicates whether the network is in ecotone mode.
/// @notice indicates whether the network has gone through the Ecotone upgrade

bool public isEcotone;

/// @notice Computes the L1 portion of the fee based on the size of the rlp encoded input
/// transaction, the current L1 base fee, and the various dynamic parameters.
/// @param _data Unsigned fully RLP-encoded transaction to get the L1 fee for.
/// @return L1 fee that should be paid for the tx
function getL1Fee(bytes memory _data) external view returns (uint256) {
uint256 l1GasUsed = getL1GasUsed(_data);
uint256 l1Fee = l1GasUsed * l1BaseFee();
uint256 divisor = 10 ** DECIMALS;
uint256 unscaled = l1Fee * scalar();
uint256 scaled = unscaled / divisor;
return scaled;
if (isEcotone) {
_getL1FeeEcotone(_data);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to return

}
return _getL1FeeBedrock(_data);
}

/// @notice Set chain to be Ecotone chain (callable by depositor account)
function setEcotone() external {
require(
msg.sender == L1Block(Predeploys.L1_BLOCK_ATTRIBUTES).DEPOSITOR_ACCOUNT(),
"GasPriceOracle: only the depositor account can set isEcotone flag"
);
isEcotone = true;
}

/// @notice Retrieves the current gas price (base fee).
Expand All @@ -52,15 +62,23 @@ contract GasPriceOracle is ISemver {
return block.basefee;
}

/// @custom:legacy
/// @notice Retrieves the current fee overhead.
/// @return Current fee overhead.
function overhead() public view returns (uint256) {
if (isEcotone) {
revert("GasPriceOracle: overhead() is deprecated");
}
return L1Block(Predeploys.L1_BLOCK_ATTRIBUTES).l1FeeOverhead();
}

/// @custom:legacy
/// @notice Retrieves the current fee scalar.
/// @return Current fee scalar.
function scalar() public view returns (uint256) {
if (isEcotone) {
revert("GasPriceOracle: scalar() is deprecated");
}
return L1Block(Predeploys.L1_BLOCK_ATTRIBUTES).l1FeeScalar();
}

Expand All @@ -70,20 +88,71 @@ contract GasPriceOracle is ISemver {
return L1Block(Predeploys.L1_BLOCK_ATTRIBUTES).basefee();
}

/// @notice Retrieves the current blob base fee.
/// @return Current blob base fee.
function blobBasefee() public view returns (uint256) {
return L1Block(Predeploys.L1_BLOCK_ATTRIBUTES).blobBasefee();
}

/// @notice Retrieves the current base fee scalar.
/// @return Current base fee scalar.
function basefeeScalar() public view returns (uint32) {
return L1Block(Predeploys.L1_BLOCK_ATTRIBUTES).basefeeScalar();
}

/// @notice Retrieves the current blob base fee scalar.
/// @return Current blob base fee scalar.
function blobBasefeeScalar() public view returns (uint32) {
return L1Block(Predeploys.L1_BLOCK_ATTRIBUTES).blobBasefeeScalar();
}

/// @custom:legacy
/// @notice Retrieves the number of decimals used in the scalar.
/// @return Number of decimals used in the scalar.
function decimals() public pure returns (uint256) {
return DECIMALS;
}

/// @notice Computes the amount of L1 gas used for a transaction. Adds the overhead which
/// represents the per-transaction gas overhead of posting the transaction and state
/// roots to L1. Adds 68 bytes of padding to account for the fact that the input does
/// not have a signature.
/// @notice Computes the amount of L1 gas used for a transaction. Adds 68 bytes
/// of padding to account for the fact that the input does not have a signature.
anikaraghu marked this conversation as resolved.
Show resolved Hide resolved
/// @param _data Unsigned fully RLP-encoded transaction to get the L1 gas for.
/// @return Amount of L1 gas used to publish the transaction.
function getL1GasUsed(bytes memory _data) public view returns (uint256) {
if (isEcotone) {
return _getL1GasUsedEcotone(_data);
}
return _getL1GasUsedBedrock(_data);
}

/// @notice Pre-ecotone computation of the L1 portion of the fee.
/// @param _data Unsigned fully RLP-encoded transaction to get the L1 fee for.
/// @return L1 fee that should be paid for the tx
function _getL1FeeBedrock(bytes memory _data) internal view returns (uint256) {
uint256 l1GasUsed = _getL1GasUsedBedrock(_data);
uint256 l1Fee = l1GasUsed * l1BaseFee();
uint256 unscaled = l1Fee * L1Block(Predeploys.L1_BLOCK_ATTRIBUTES).l1FeeScalar();
uint256 divisor = 10 ** DECIMALS;
uint256 scaled = unscaled / divisor;
return scaled;
}

/// @notice Post-ecotone computation of the L1 portion of the fee.
/// @param _data Unsigned fully RLP-encoded transaction to get the L1 fee for.
/// @return L1 fee that should be paid for the tx
function _getL1FeeEcotone(bytes memory _data) internal view returns (uint256) {
uint256 l1GasUsed = _getL1GasUsedEcotone(_data);
uint256 scaledBasefee = basefeeScalar() * 16 * l1BaseFee();
uint256 scaledBlobBasefee = blobBasefeeScalar() * blobBasefee();
uint256 unscaled = l1GasUsed * (scaledBasefee + scaledBlobBasefee);
uint256 divisor = 10 ** DECIMALS;
uint256 scaled = unscaled / divisor;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to move the division by 16 to the end to preserve precision.
If we change l1GasUsed as suggested above:

Suggested change
uint256 l1GasUsed = _getL1GasUsedEcotone(_data);
uint256 scaledBasefee = basefeeScalar() * 16 * l1BaseFee();
uint256 scaledBlobBasefee = blobBasefeeScalar() * blobBasefee();
uint256 unscaled = l1GasUsed * (scaledBasefee + scaledBlobBasefee);
uint256 divisor = 10 ** DECIMALS;
uint256 scaled = unscaled / divisor;
uint256 l1GasUsed = _getL1GasUsed(_data);
uint256 scaledBasefee = basefeeScalar() * 16 * l1BaseFee();
uint256 scaledBlobBasefee = blobBasefeeScalar() * blobBasefee();
uint256 unscaled = l1GasUsed * (scaledBasefee + scaledBlobBasefee);
uint256 divisor = 16 * 10 ** DECIMALS;
uint256 scaled = unscaled / divisor;

return scaled;
}

/// @notice Pre-ecotone L1 gas estimation calculation.
/// @param _data Unsigned fully RLP-encoded transaction to get the L1 gas for.
/// @return Amount of L1 gas used to publish the transaction.
function _getL1GasUsedBedrock(bytes memory _data) internal view returns (uint256) {
uint256 total = 0;
uint256 length = _data.length;
for (uint256 i = 0; i < length; i++) {
Expand All @@ -93,7 +162,23 @@ contract GasPriceOracle is ISemver {
total += 16;
}
}
uint256 unsigned = total + overhead();
uint256 unsigned = total + L1Block(Predeploys.L1_BLOCK_ATTRIBUTES).l1FeeOverhead();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
uint256 unsigned = total + L1Block(Predeploys.L1_BLOCK_ATTRIBUTES).l1FeeOverhead();
uint256 unsigned = total + isEcotone ? 0 : L1Block(Predeploys.L1_BLOCK_ATTRIBUTES).l1FeeOverhead();

...From Slack discussion where the proposal is to just make this return "calldataGas".

then you can drop the _getL1GasUsedEcotone() function entirely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

worth noting that with this approach the _getL1FeeBedrock / _getL1FeeEcotone methods are checking the isEcotone flag twice. are we ok with that additional gas usage?

return unsigned + (68 * 16);
}

/// @notice Post-ecotone L1 gas estimation calculation.
/// @param _data Unsigned fully RLP-encoded transaction to get the L1 gas for.
/// @return Amount of L1 gas used to publish the transaction.
function _getL1GasUsedEcotone(bytes memory _data) internal pure returns (uint256) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't computing a "gas" value, but the approximation of "compressed bytes", so I'd rename it something more accurate to make it better match the spec.

_getCompressedTxSize() ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from Slack discussion: just delete this function after applying the suggested edit above to the "main" function.

uint256 total = 0;
uint256 length = _data.length;
for (uint256 i = 0; i < length; i++) {
if (_data[i] == 0) {
total += 4;
} else {
total += 16;
}
}
return (total + (68 * 16 * 16)) / 16;
Copy link
Collaborator

@roberto-bayardo roberto-bayardo Dec 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return (total + (68 * 16 * 16)) / 16;
return (total / 16) + 68

}
}
74 changes: 72 additions & 2 deletions packages/contracts-bedrock/src/L2/L1Block.sol
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,26 @@ contract L1Block is ISemver {
bytes32 public batcherHash;

/// @notice The overhead value applied to the L1 portion of the transaction fee.
/// @custom:legacy
uint256 public l1FeeOverhead;

/// @notice The scalar value applied to the L1 portion of the transaction fee.
/// @custom:legacy
uint256 public l1FeeScalar;

/// @custom:semver 1.1.0
string public constant version = "1.1.0";
/// @notice The latest L1 blob basefee.
uint256 public blobBasefee;

/// @notice The scalar value applied to the L1 base fee portion of the blob-capable L1 cost func
uint32 public basefeeScalar;

/// @notice The scalar value applied to the L1 blob base fee portion of the blob-capable L1 cost func
uint32 public blobBasefeeScalar;

/// @custom:semver 1.2.0
string public constant version = "1.2.0";

/// @custom:legacy
/// @notice Updates the L1 block values.
/// @param _number L1 blocknumber.
/// @param _timestamp L1 timestamp.
Expand Down Expand Up @@ -73,4 +85,62 @@ contract L1Block is ISemver {
l1FeeOverhead = _l1FeeOverhead;
l1FeeScalar = _l1FeeScalar;
}

/// @notice Updates the L1 block values for a post-blob activated chain.
/// Params are passed in as part of msg.data in order to compress the calldata.
/// Params should be passed in in the following order:
/// 1. _basefeeScalar L1 base fee scalar
/// 2. _blobBasefeeScalar L1 blob base fee scalar
/// 3. _sequenceNumber Number of L2 blocks since epoch start.
/// 4. _timestamp L1 timestamp.
/// 5. _number L1 blocknumber.
/// 6. _basefee L1 basefee.
/// 7. _blobBasefee L1 blobBasefee.
/// 8. _hash L1 blockhash.
/// 9. _batcherHash Versioned hash to authenticate batcher by.
function setL1BlockValuesEcotone() external {
require(msg.sender == DEPOSITOR_ACCOUNT, "L1Block: only the depositor account can set L1 block values");

uint256 _basefeeScalar;
uint256 _blobBasefeeScalar;
uint256 _sequenceNumber;
uint256 _timestamp;
uint256 _number;
uint256 _basefee;
uint256 _blobBasefee;
bytes32 _hash;
bytes32 _batcherHash;

assembly {
let offset := 0x4
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think having an offset for first pass to get passing tests is fine but ideally we don't need to have the intermediate adds and we just hardcode in the values. We should also be able to sstore directly in the yul but I am fine with that as a nice to have

_basefeeScalar := shr(224, calldataload(offset)) // uint32
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the basefeescalar and the blobbasefeescalar we technically do not need to even parse them into their own values on the stack, since they are next to each other in the calldata and in storage we can sstore(slot, shr(160, calldataload(4)) or something like that

offset := add(offset, 0x4)
_blobBasefeeScalar := shr(224, calldataload(offset)) // uint32
offset := add(offset, 0x4)
_sequenceNumber := shr(192, calldataload(offset)) // uint64
offset := add(offset, 0x8)
_timestamp := shr(192, calldataload(offset)) // uint64
offset := add(offset, 0x8)
_number := shr(192, calldataload(offset)) // uint64
offset := add(offset, 0x8)
_basefee := calldataload(offset) // uint256
offset := add(offset, 0x20)
_blobBasefee := calldataload(offset) // uint256
offset := add(offset, 0x20)
_hash := calldataload(offset) // bytes32
offset := add(offset, 0x20)
_batcherHash := calldataload(offset) // bytes32
offset := add(offset, 0x20)
}

number = uint64(_number);
timestamp = uint64(_timestamp);
basefee = _basefee;
blobBasefee = _blobBasefee;
hash = _hash;
sequenceNumber = uint64(_sequenceNumber);
batcherHash = _batcherHash;
basefeeScalar = uint32(_basefeeScalar);
blobBasefeeScalar = uint32(_blobBasefeeScalar);
}
}
Loading