SHIP-2006 Consolidate All FinalityDepth Settings to default TOML files #1151
Conversation
…thub.com:smartcontractkit/ccip into SHIP-1754-Consolidate-all-FinalityDepth-settings
…thub.com:smartcontractkit/ccip into SHIP-1754-Consolidate-all-FinalityDepth-settings
|
@simsonraj @stackman27 What do you guys think? @friedemannf suggested that we should only rely on custom settings from:
|
|
Closing this to prevent reviews. |
…-testnet overrides
|
Opening with update values for testnet, relying only on prod-testnet infra-k8s overrides |
|
The latest commit added the main net values set by CCIP Node Setup Guide |
| @@ -1,5 +1,5 @@ | |||
| ChainID = '59141' | |||
| FinalityDepth = 900 | |||
| FinalityDepth = 200 | |||
There was a problem hiding this comment.
why 200 on Sepolia but 600 on Mainnet?
There was a problem hiding this comment.
200 is the testnet prod value
200 introduced by @stackman27 at https://github.com/smartcontractkit/infra-k8s/pull/20027
I have no idea why Mainnet is set to 600, I'm following what is oriented on CCIP Node Setup Guide
Maybe the block time is different between testnet and mainnet and that's why?
| @@ -2,7 +2,7 @@ | |||
| ChainID = '1088' | |||
| ChainType = 'metis' | |||
| # Sequencer offers absolute finality | |||
| FinalityDepth = 10 | |||
| FinalityDepth = 500 | |||
There was a problem hiding this comment.
Also difference between mainnet & sepolia
There was a problem hiding this comment.
Wow... nice find. I think Aditya introduced this and slipped my revision.
There is no FinalityDepth value set for Metis (testnet or mainnet)... Do you think I should remove these, then?
| @@ -1,23 +1,23 @@ | |||
| ChainID = '300' | |||
| ChainType = 'zksync' | |||
| # 200block ~ 20min concurrent with the l1_committed tag | |||
| FinalityDepth = 200 | |||
| FinalityDepth = 200 | |||
There was a problem hiding this comment.
Unless the block timing is different between mainnet & testnet, can you please also give a pass on all the mainnet configs ?
There was a problem hiding this comment.
I am getting these values from infra-k8s prod-testnet overrides.
Do you think there's a chance that those values are also wrong?
https://github.com/smartcontractkit/infra-k8s/blob/a7e46c641e13a3ae32b62f598f7c49dc913bd0ad/projects/chainlink/files/chainlink-clusters/cl-ccip-testnets-prod/config.yaml#L662
[[EVM]]
ChainID = '300'
ChainType = 'zksync'
FinalityTagEnabled = false
# 200block ~ 20min concurrent with the l1_committed tag
FinalityDepth = 200
# block rate is ~2-5sec, so this ensures blocks are polled correctly
LogPollInterval = '5s'
# sufficient time for RPC to be labelled out of sync, since blockRate is pretty fast
NoNewHeadsThreshold = '1m'
[EVM.GasEstimator]
# no EIP1559 to ensure our estimator does not estimate gas with MaxPriorityFee which will break minFunding requirement
EIP1559DynamicFees = false
# high LimitDefault for worst case pubdata bytes with BatchGasLimit reduced to 4M in OCR2Config
LimitDefault = 2_500_000_000
FeeCapDefault = '500 mwei'
PriceDefault = '25 mwei'
# p999 value for gasPrice based on historical data
PriceMax = '500 mwei'
# avg gasPrices are at 0.025 gwei
PriceMin = '25 mwei'
[EVM.GasEstimator.BlockHistory]
# increasing this to smooth out gas estimation
BlockHistorySize = 200
[EVM.HeadTracker]
# tracks top N blocks to keep in heads database table. Should store atleast the same # of blocks as finalityDepth
HistoryDepth = 250
There was a problem hiding this comment.
The finality committee only really signed off on mainnets, but IMO we should set the same values for testnets as well
|
Motivation
Ticket
Solution
Updated the values of FinalityDepth setting to make it the same as the values set in infra-k8s repo.