Charge l1 data availability fee #86

matYang · 2023-08-28T04:34:07Z

Motivation

When sending to supported L2 rollups, we undercharge the fee as execution gas cost only represents tiny portion of the fee. Most of the cost at destination comes from L1 security fee.

Solution

Currently supported L2 rollups are optimistic rollups. Their L1 security fee can be estimated with equation

txCost = l2_gasPrice * l2_gasUsed // L2 EVM execution cost
  + l1_base_fee // set by the L2
    * (length_in_bytes(tx_data) * gas_per_byte + fixed_overhead)
    * dynamic_compression_factor // L1 calldata cost

github-actions · 2023-08-28T04:38:39Z

LCOV of commit `18d44e2` during Solidity Foundry #329

Summary coverage rate:
  lines......: 0.0% (0 of 909 lines)
  functions..: 0.0% (0 of 199 functions)
  branches...: 0.0% (0 of 386 branches)

Files changed coverage rate: n/a

⛔ The code coverage is too low: 0. Expected at least 98.

github-actions · 2023-08-28T04:39:55Z

Go solidity wrappers are out-of-date, regenerate them via the make wrappers-all command

RensR · 2023-08-28T14:04:12Z

contracts/src/v0.8/ccip/libraries/Internal.sol

+
+  // little endian
+  function _unpackUint224Into112(uint224 value) internal pure returns (uint112, uint112) {
+    return (uint112(value & UINT_112__MASK), uint112((value >> 112) & UINT_112__MASK));


Do you need the mask or is downcasting enough already?

good point, downcasting truncates higher order bits, that'll just work

github-actions · 2023-08-29T21:32:31Z

LCOV of commit `3b6da8a` during Solidity Foundry #378

Summary coverage rate:
  lines......: 0.0% (0 of 894 lines)
  functions..: 0.0% (0 of 194 functions)
  branches...: 0.0% (0 of 378 branches)

Files changed coverage rate: n/a

⛔ The code coverage is too low: 0. Expected at least 98.

github-actions · 2023-08-29T21:35:49Z

Go solidity wrappers are out-of-date, regenerate them via the make wrappers-all command

github-actions · 2023-08-29T22:40:50Z

LCOV of commit `3bdab64` during Solidity Foundry #380

Summary coverage rate:
  lines......: 0.0% (0 of 894 lines)
  functions..: 0.0% (0 of 194 functions)
  branches...: 0.0% (0 of 378 branches)

Files changed coverage rate: n/a

⛔ The code coverage is too low: 0. Expected at least 98.

github-actions · 2023-08-29T22:44:10Z

Go solidity wrappers are out-of-date, regenerate them via the make wrappers-all command

github-actions · 2023-08-30T04:38:57Z

LCOV of commit `2079ffb` during Solidity Foundry #387

Summary coverage rate:
  lines......: 98.3% (883 of 898 lines)
  functions..: 95.9% (187 of 195 functions)
  branches...: 90.5% (342 of 378 branches)

Files changed coverage rate: n/a

github-actions · 2023-08-30T04:41:17Z

Go solidity wrappers are out-of-date, regenerate them via the make wrappers-all command

github-actions · 2023-08-30T06:13:42Z

LCOV of commit `bc6e602` during Solidity Foundry #388

Summary coverage rate:
  lines......: 98.9% (888 of 898 lines)
  functions..: 96.4% (188 of 195 functions)
  branches...: 90.7% (343 of 378 branches)

Files changed coverage rate: n/a

github-actions · 2023-08-30T06:15:16Z

Go solidity wrappers are out-of-date, regenerate them via the make wrappers-all command

github-actions · 2023-08-30T19:40:03Z

LCOV of commit `8465a72` during Solidity Foundry #398

Summary coverage rate:
  lines......: 98.9% (888 of 898 lines)
  functions..: 96.4% (188 of 195 functions)
  branches...: 90.7% (343 of 378 branches)

Files changed coverage rate: n/a

RensR · 2023-09-01T16:46:50Z

contracts/src/v0.8/ccip/onRamp/EVM2EVMOnRamp.sol

+    uint16 maxTokensLength; //           | Maximum number of ERC20 token transfers that can be included per message
+    uint32 destGasOverhead; //           | Extra gas charged on top of the gasLimit
+    uint16 destGasPerPayloadByte; //     | Destination chain gas charged per byte of `data` payload
+    uint32 destCalldataOverhead; //      | Extra L1 calldata gas on top of the message used in security fee


let's add more info here, because L1 doesn't make sense for any L1. Also, i've never heard the term "security fee"

I've heard of security fee, but as far as nomenclature goes I think DA (data availability) is the most general purpose/generic and we should use it everywhere. L2's happen to store the data as calldata, but need not, its really just about paying a fee to ensure the data is made available. Should be able to leave out the notion of L1/L2's all together as well

RensR · 2023-09-01T16:47:08Z

contracts/src/v0.8/ccip/onRamp/EVM2EVMOnRamp.sol

+    uint32 destGasOverhead; //           | Extra gas charged on top of the gasLimit
+    uint16 destGasPerPayloadByte; //     | Destination chain gas charged per byte of `data` payload
+    uint32 destCalldataOverhead; //      | Extra L1 calldata gas on top of the message used in security fee
+    uint16 destGasPerCalldataByte; // ---┘ Number of L1 calldata gas to charge per byte of message


nit: amount of instead of number of

RensR · 2023-09-01T17:02:56Z

core/services/ocr2/plugins/ccip/plugins_common.go

+			return evm_2_evm_onramp.EVM2EVMOnRampDynamicConfig{}, err
+		}
+		return evm_2_evm_onramp.EVM2EVMOnRampDynamicConfig{
+			Router:          legacyDynamicConfig.Router,


You're missing some values that are in 1.1

RensR · 2023-09-01T17:04:10Z

integration-tests/ccip-tests/load/ccip_loadgen.go

@@ -39,7 +39,7 @@ type CCIPE2ELoad struct {
 	CallTimeOut               time.Duration // max time to wait for various on-chain events
 	reports                   *testreporters.CCIPLaneStats
 	msg                       router.ClientEVM2AnyMessage
-	MaxDataSize               uint32
+	MaxDataSize               *big.Int


Is this big int because go doesn' have uint24?

I think so.
uint16 gives us 64KB, that's not enough, Ethereum can do >120KB.
uint32 gives us 4GB, that's seems a bit too big.
so went with uint24, and it become big.int in generated code

connorwstein · 2023-09-01T19:50:51Z

contracts/src/v0.8/ccip/PriceRegistry.sol

@@ -34,15 +34,15 @@ contract PriceRegistry is IPriceRegistry, OwnerIsCreator {
  ///     Very Expensive:   1 unit of gas costs 1 USD                  -> 1e18
  ///     Expensive:        1 unit of gas costs 0.1 USD                -> 1e17
  ///     Cheap:            1 unit of gas costs 0.000001 USD           -> 1e12
-  mapping(uint64 destChainSelector => Internal.TimestampedUint192Value price)
+  mapping(uint64 destChainSelector => Internal.TimestampedPackedUint224 price)


comments need updating

connorwstein · 2023-09-01T20:19:38Z

contracts/src/v0.8/ccip/interfaces/IPriceRegistry.sol


  /// @notice Get the `tokenPrice` for an array of tokens.
  /// @param tokens The tokens to get prices for.
  /// @return tokenPrices The tokenPrices for the given tokens.
-  function getTokenPrices(address[] calldata tokens) external view returns (Internal.TimestampedUint192Value[] memory);
+  function getTokenPrices(address[] calldata tokens) external view returns (Internal.TimestampedPackedUint224[] memory);

  /// @notice Get the `gasPrice` for a given destination chain ID.
  /// @param destChainSelector The destination chain to get the price for.
  /// @return gasPrice The gasPrice for the given destination chain ID.
  function getDestinationChainGasPrice(


probably update the comment here? it now returns a multi-dimensional gas price

connorwstein · 2023-09-01T20:28:11Z

contracts/src/v0.8/ccip/libraries/Internal.sol

  }

+  /// @dev Gas price is stored in 112-bit unsigned int. uint224 can pack 2 prices.


Can we abstract this from price reg clients all together? Since the price reg API is breaking anyways, something like getValidatedExecutionPrices(destinationSelector)->(gasPrice, daPrice) where the 2 return values are already parsed for the client. Also that way I think we don't even need the multiplier check (if daPrice == 0, then DA is free/covered by gasPrice)

spoke offline - I'm cool with opaque gas price and ramps interpreting

connorwstein · 2023-09-01T20:28:56Z

contracts/src/v0.8/ccip/PriceRegistry.sol

@@ -13,7 +13,7 @@ import {EnumerableSet} from "../vendor/openzeppelin-solidity/v4.8.0/contracts/ut
 /// and the price of a token in USD allowing the owner or priceUpdater to update this value.
 contract PriceRegistry is IPriceRegistry, OwnerIsCreator {
  using EnumerableSet for EnumerableSet.AddressSet;
-  using USDPriceWith18Decimals for uint192;


great opportunity to add a type and version to the price reg

connorwstein · 2023-09-01T20:31:03Z

contracts/src/v0.8/ccip/PriceRegistry.sol

-  ) external view override returns (uint192 tokenPrice, uint192 gasPriceValue) {
-    Internal.TimestampedUint192Value memory gasPrice = s_usdPerUnitGasByDestChainSelector[destChainSelector];
+  ) external view override returns (uint224 tokenPrice, uint224 gasPriceValue) {
+    Internal.TimestampedPackedUint224 memory gasPrice = s_usdPerUnitGasByDestChainSelector[destChainSelector];


thoughts on an internal _getValidatedExecutionPrices analogous to the _getValidatedTokenPrice? Yields some nice symmetry validatedXX means timestamp checked for both gas and tokens

will include in a follow up PR that includes _getValidatedGasPrices and getValidatedExecutionPrices

connorwstein · 2023-09-01T20:35:16Z

contracts/src/v0.8/ccip/onRamp/EVM2EVMOnRamp.sol

+    uint16 maxTokensLength; //           | Maximum number of ERC20 token transfers that can be included per message
+    uint32 destGasOverhead; //           | Extra gas charged on top of the gasLimit
+    uint16 destGasPerPayloadByte; //     | Destination chain gas charged per byte of `data` payload
+    uint32 destCalldataOverhead; //      | Extra L1 calldata gas on top of the message used in security fee


I've heard of security fee, but as far as nomenclature goes I think DA (data availability) is the most general purpose/generic and we should use it everywhere. L2's happen to store the data as calldata, but need not, its really just about paying a fee to ensure the data is made available. Should be able to leave out the notion of L1/L2's all together as well

connorwstein · 2023-09-01T20:37:44Z

contracts/src/v0.8/ccip/libraries/Internal.sol

@@ -9,19 +9,24 @@ library Internal {
  struct PriceUpdates {
    TokenPriceUpdate[] tokenPriceUpdates;
    uint64 destChainSelector; // --┐ Destination chain selector
-    uint192 usdPerUnitGas; // -----┘ 1e18 USD per smallest unit (e.g. wei) of destination chain gas
+    uint224 usdPerUnitGas; // -----┘ 1e18 USD per smallest unit (e.g. wei) of destination chain gas


while we're changing this interface, we should open the door for batched gas price updates i.e. have a GasPriceUpdate[] where GasPriceUpdate{DestChainSelector, GasPrice}. Can be a follow up if painful to add now

As long as it is in 1.2 and doesn't bleed into 1.3 I don't mind doing it in follow up

I'll post it in a followup PR, this is PR is already quite big.

connorwstein · 2023-09-01T20:38:43Z

contracts/src/v0.8/ccip/libraries/Internal.sol

+  /// Each variable array takes 1 more slot to store its length.
+  /// Hence, when abiEncoded, excluding the dynamic contents of variable arrays,
+  /// EVM2EVMMessage takes up a fixed number of 14 lots, 32 bytes each.
+  uint256 public constant MESSAGE_FIXED_BYTES = 32 * 14;


hmm I'm ok with it here as its a tightly coupled property of EVM2EVMMessage

connorwstein · 2023-09-01T20:41:46Z

contracts/src/v0.8/ccip/onRamp/EVM2EVMOnRamp.sol

+    uint256 numberOfTokens,
+    uint32 tokenTransferBytesOverhead
+  ) internal view returns (uint256 calldataCostUSD) {
+    uint256 calldataLength = Internal.MESSAGE_FIXED_BYTES +


would use units suffixes everywhere i.e. calldataLengthBytes, s_dynamicConfig.destCalldataOverheadGas etc

github-actions · 2023-09-06T22:48:41Z

LCOV of commit `125f004` during Solidity Foundry #454

Summary coverage rate:
  lines......: 98.9% (909 of 919 lines)
  functions..: 96.4% (189 of 196 functions)
  branches...: 91.1% (357 of 392 branches)

Files changed coverage rate: n/a

RensR · 2023-09-07T13:21:17Z

contracts/src/v0.8/ccip/PriceRegistry.sol

@@ -29,20 +30,27 @@ contract PriceRegistry is IPriceRegistry, OwnerIsCreator {
  event UsdPerUnitGasUpdated(uint64 indexed destChain, uint256 value, uint256 timestamp);
  event UsdPerTokenUpdated(address indexed token, uint256 value, uint256 timestamp);

-  /// @dev The price, in USD with 18 decimals, of 1 unit of gas for a given destination chain.
+  // STATIC CONFIG


not sure we need this header for a single item

RensR · 2023-09-07T13:23:50Z

contracts/src/v0.8/ccip/interfaces/IPriceRegistry.sol

-  /// @notice Get the `gasPrice` for a given destination chain ID.
+  /// @notice Get an encoded `gasPrice` for a given destination chain ID.
+  /// The 224-bit result could encode multiple price components. For example, if Optimism is the destination chain,
+  /// gas price can include L1 base fee in higher-order bits, and L2 gas price in lower-order bits.


nit: gas price can include
it's not can, it always will, right? If not explain when this is and isnt the case

RensR · 2023-09-07T13:25:42Z

contracts/src/v0.8/ccip/onRamp/EVM2EVMOnRamp.sol

+    uint16 destGasPerDataAvailabilityByte; // --┘ Amount of gas to charge per byte of data that needs availability
+    uint16 destDataAvailabilityMultiplier; // --┐ Multiplier for data availability gas, multples of 1e-4, or 0.0001
+    address priceRegistry; //                   | Price registry address
+    uint24 maxDataSize; //                      | Maximum payload data size, max 16MB


Let's make this uint32 as it fits and 24 is a strange datatype that becomes big.int offchain

github-actions · 2023-09-07T16:29:44Z

LCOV of commit `178c6f8` during Solidity Foundry #468

Summary coverage rate:
  lines......: 98.9% (909 of 919 lines)
  functions..: 96.4% (189 of 196 functions)
  branches...: 91.1% (357 of 392 branches)

Files changed coverage rate: n/a

github-actions · 2023-09-09T06:30:06Z

LCOV of commit `6e2be8f` during Solidity Foundry #481

Summary coverage rate:
  lines......: 98.9% (909 of 919 lines)
  functions..: 96.4% (189 of 196 functions)
  branches...: 91.1% (357 of 392 branches)

Files changed coverage rate: n/a

github-actions · 2023-09-11T14:42:05Z

LCOV of commit `6807c01` during Solidity Foundry #482

Summary coverage rate:
  lines......: 98.9% (909 of 919 lines)
  functions..: 96.4% (189 of 196 functions)
  branches...: 91.1% (357 of 392 branches)

Files changed coverage rate: n/a

github-actions · 2023-09-11T15:13:50Z

LCOV of commit `e813a07` during Solidity Foundry #483

Summary coverage rate:
  lines......: 98.9% (909 of 919 lines)
  functions..: 96.4% (189 of 196 functions)
  branches...: 91.1% (357 of 392 branches)

Files changed coverage rate: n/a

github-actions · 2023-09-11T17:54:25Z

LCOV of commit `7f22a73` during Solidity Foundry #488

Summary coverage rate:
  lines......: 98.9% (909 of 919 lines)
  functions..: 96.4% (189 of 196 functions)
  branches...: 91.1% (357 of 392 branches)

Files changed coverage rate: n/a

* add uint224 price split and call data config fields * add calldata calculation * address early comments * add more comments to explain config values * completing onramp * fill in transfer fee tests * add tests for getMessageCalldataCostUSD * onchain tests complete * complete onchain portion * update offchain tests * update snapshot * fix loadgen test MaxDataSize * better variable names * update changelog * address comments * address comments * re-generate snapshot * update loadgen * reset submodule * update snapshot

matYang added 2 commits August 27, 2023 02:30

add uint224 price split and call data config fields

e785dc8

add calldata calculation

18d44e2

matYang requested a review from connorwstein August 28, 2023 04:34

matYang changed the title ~~Feature/charge l1 security fee~~ [WIP] Charge l1 security fee Aug 28, 2023

RensR reviewed Aug 28, 2023

View reviewed changes

matYang added 2 commits August 28, 2023 15:44

address early comments

c839e83

Merge branch 'ccip-develop' into feature/charge-l1-security-fee

3b6da8a

add more comments to explain config values

3bdab64

completing onramp

2079ffb

matYang added 2 commits August 30, 2023 01:19

fill in transfer fee tests

2e9f4bc

add tests for getMessageCalldataCostUSD

bc6e602

matYang added 2 commits August 30, 2023 12:20

onchain tests complete

bb5900c

complete onchain portion

8465a72

update offchain tests

ff719db

matYang changed the title ~~[WIP] Charge l1 security fee~~ Charge l1 security fee Aug 30, 2023

matYang added 2 commits August 30, 2023 17:56

Merge branch 'ccip-develop' into feature/charge-l1-security-fee

c80a8fd

update snapshot

f6cca92

matYang marked this pull request as ready for review August 30, 2023 22:05

matYang requested a review from a team as a code owner August 30, 2023 22:05

RensR reviewed Sep 1, 2023

View reviewed changes

connorwstein reviewed Sep 1, 2023

View reviewed changes

Merge branch 'ccip-develop' into feature/charge-l1-security-fee

dbe18ba

matYang changed the title ~~Charge l1 security fee~~ Charge l1 data availability fee Sep 6, 2023

address comments

125f004

matYang requested review from RensR and connorwstein September 6, 2023 22:43

RensR reviewed Sep 7, 2023

View reviewed changes

address comments

178c6f8

RensR self-requested a review September 7, 2023 21:19

RensR approved these changes Sep 7, 2023

View reviewed changes

matYang added 2 commits September 9, 2023 02:20

Merge branch 'ccip-develop' into feature/charge-l1-security-fee

be9281c

re-generate snapshot

6e2be8f

update loadgen

6807c01

reset submodule

e813a07

update snapshot

7f22a73

AnieeG approved these changes Sep 11, 2023

View reviewed changes

matYang merged commit e41e93a into ccip-develop Sep 11, 2023
71 checks passed

matYang deleted the feature/charge-l1-security-fee branch September 11, 2023 19:15

chainchad pushed a commit to chainchad/ccip that referenced this pull request Jan 24, 2024

deploy GHO & wemix/kroma test tokens (smartcontractkit#86)

ab741ec

		}

		/// @dev Gas price is stored in 112-bit unsigned int. uint224 can pack 2 prices.

Charge l1 data availability fee #86

Charge l1 data availability fee #86

Conversation

matYang commented Aug 28, 2023 • edited Loading

Motivation

Solution

github-actions bot commented Aug 28, 2023

LCOV of commit 18d44e2 during Solidity Foundry #329

github-actions bot commented Aug 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Aug 29, 2023

LCOV of commit 3b6da8a during Solidity Foundry #378

github-actions bot commented Aug 29, 2023

github-actions bot commented Aug 29, 2023

LCOV of commit 3bdab64 during Solidity Foundry #380

github-actions bot commented Aug 29, 2023

github-actions bot commented Aug 30, 2023

LCOV of commit 2079ffb during Solidity Foundry #387

github-actions bot commented Aug 30, 2023

github-actions bot commented Aug 30, 2023

LCOV of commit bc6e602 during Solidity Foundry #388

github-actions bot commented Aug 30, 2023

github-actions bot commented Aug 30, 2023

LCOV of commit 8465a72 during Solidity Foundry #398

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 6, 2023

LCOV of commit 125f004 during Solidity Foundry #454

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 7, 2023

LCOV of commit 178c6f8 during Solidity Foundry #468

github-actions bot commented Sep 9, 2023

LCOV of commit 6e2be8f during Solidity Foundry #481

github-actions bot commented Sep 11, 2023

LCOV of commit 6807c01 during Solidity Foundry #482

github-actions bot commented Sep 11, 2023

LCOV of commit e813a07 during Solidity Foundry #483

github-actions bot commented Sep 11, 2023

LCOV of commit 7f22a73 during Solidity Foundry #488

matYang commented Aug 28, 2023 •

edited

Loading

LCOV of commit `18d44e2` during Solidity Foundry #329

LCOV of commit `3b6da8a` during Solidity Foundry #378

LCOV of commit `3bdab64` during Solidity Foundry #380

LCOV of commit `2079ffb` during Solidity Foundry #387

LCOV of commit `bc6e602` during Solidity Foundry #388

LCOV of commit `8465a72` during Solidity Foundry #398

LCOV of commit `125f004` during Solidity Foundry #454

LCOV of commit `178c6f8` during Solidity Foundry #468

LCOV of commit `6e2be8f` during Solidity Foundry #481

LCOV of commit `6807c01` during Solidity Foundry #482

LCOV of commit `e813a07` during Solidity Foundry #483

LCOV of commit `7f22a73` during Solidity Foundry #488