Closed dantaik closed 1 year ago
Merging #12989 (861a65d) into main (16993cd) will increase coverage by
0.47%
. The diff coverage isn/a
.:exclamation: Current head 861a65d differs from pull request most recent head 0d36bcf. Consider uploading reports for the commit 0d36bcf to get more accurate results
@@ Coverage Diff @@
## main #12989 +/- ##
==========================================
+ Coverage 60.19% 60.67% +0.47%
==========================================
Files 115 115
Lines 3442 3415 -27
Branches 466 463 -3
==========================================
Hits 2072 2072
+ Misses 1283 1256 -27
Partials 87 87
Flag | Coverage Δ | *Carryforward flag | |
---|---|---|---|
bridge-ui | 92.61% <ø> (ø) |
Carriedforward from acfa27d | |
protocol | 51.88% <ø> (+0.76%) |
:arrow_up: | |
relayer | 65.15% <ø> (ø) |
Carriedforward from acfa27d | |
ui | 100.00% <ø> (ø) |
Carriedforward from acfa27d |
*This pull request uses carry forward flags. Click here to find out more.
Impacted Files | Coverage Δ | |
---|---|---|
...ckages/protocol/contracts/libs/LibSharedConfig.sol | 0.00% <ø> (ø) |
|
packages/protocol/contracts/L1/libs/LibUtils.sol | 32.20% <0.00%> (-0.56%) |
:arrow_down: |
...ackages/protocol/contracts/test/L1/TestTaikoL1.sol | 87.87% <0.00%> (-0.36%) |
:arrow_down: |
packages/protocol/contracts/libs/LibZKP.sol | 0.00% <0.00%> (ø) |
|
packages/protocol/contracts/L1/ProofVerifier.sol | 0.00% <0.00%> (ø) |
|
packages/protocol/contracts/L1/libs/LibProving.sol | 0.00% <0.00%> (ø) |
|
...ackages/protocol/contracts/test/L1/TestTaikoL2.sol | 0.00% <0.00%> (ø) |
|
...es/protocol/contracts/test/libs/TestLibProving.sol | 0.00% <0.00%> (ø) |
|
.../contracts/test/L1/TestTaikoL1EnableTokenomics.sol | 0.00% <0.00%> (ø) |
|
...cts/test/L1/TestTaikoL2EnablePublicInputsCheck.sol | 100.00% <0.00%> (ø) |
|
... and 3 more |
:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more
There are some other configuration files in k8s-config
when deploying our different networks. So this LibSharedConfig.sol
file is not used.
Changed maxVerificationsPerTx
to 10
. According to the test, each time one more block is verified, there will be ~20k more gas cost, and the base cost of the verification process is ~90K, so i think verifying extra 10 blocks (~25K extra gas cost) is more acceptable than 20 blocks(~47K extra gas cost) when proposing / proving blocks:
{
method: 'verifyBlock',
nums: 1,
gasUsed: BigNumber { value: "92590" }
}
{
method: 'verifyBlock',
nums: 2,
gasUsed: BigNumber { value: "85522" }
}
{
method: 'verifyBlock',
nums: 3,
gasUsed: BigNumber { value: "105052" }
}
{
method: 'verifyBlock',
nums: 4,
gasUsed: BigNumber { value: "127941" }
}
{
method: 'verifyBlock',
nums: 5,
gasUsed: BigNumber { value: "149151" }
}
{
method: 'verifyBlock',
nums: 6,
gasUsed: BigNumber { value: "168680" }
}
{
method: 'verifyBlock',
nums: 7,
gasUsed: BigNumber { value: "191570" }
}
{
method: 'verifyBlock',
nums: 8,
gasUsed: BigNumber { value: "212780" }
}
{
method: 'verifyBlock',
nums: 9,
gasUsed: BigNumber { value: "232309" }
}
{
method: 'verifyBlock',
nums: 10,
gasUsed: BigNumber { value: "255199" }
}
{
method: 'verifyBlock',
nums: 11,
gasUsed: BigNumber { value: "276408" }
}
{
method: 'verifyBlock',
nums: 12,
gasUsed: BigNumber { value: "295938" }
}
{
method: 'verifyBlock',
nums: 13,
gasUsed: BigNumber { value: "321852" }
}
{
method: 'verifyBlock',
nums: 14,
gasUsed: BigNumber { value: "343597" }
}
{
method: 'verifyBlock',
nums: 15,
gasUsed: BigNumber { value: "363467" }
}
{
method: 'verifyBlock',
nums: 16,
gasUsed: BigNumber { value: "386616" }
}
{
method: 'verifyBlock',
nums: 17,
gasUsed: BigNumber { value: "408086" }
}
{
method: 'verifyBlock',
nums: 18,
gasUsed: BigNumber { value: "427876" }
}
{
method: 'verifyBlock',
nums: 19,
gasUsed: BigNumber { value: "451025" }
}
{
method: 'verifyBlock',
nums: 20,
gasUsed: BigNumber { value: "479507" }
}
Updated maxTransactionsPerBlock
to 79
(+ 1 TaikoL2.anchor
= 80
), since its the upper limit of @smtmfft's current circuits, and updated blockMaxGasLimit
to 6M, which is also a safe number for @smtmfft's current circuits.
For maxBytesPerTxList
, currently there is a discussion in discord: https://discord.com/channels/984015101017346058/985743259802402856/1074904085326745780 , maybe we can not do much about this, but set a value which is a little bit smaller than 128K?
For
maxBytesPerTxList
, currently there is a discussion in discord: https://discord.com/channels/984015101017346058/985743259802402856/1074904085326745780 , maybe we can not do much about this, but set a value which is a little bit smaller than 128K?
Update maxBytesPerTxList
to 120KB, since 128KB is the upper limit of a geth transaction, so using 120KB for the L1 transaction list calldata, 8K for the remaining tx fields.
This means we may have maximun 3~4 contract creations (24KB size limit), or 20 bridges (~6KB), or 79 transfers (maxTransactionsPerBlock) in one L2 block.
@davidtaikocha could you please add some of the above rationals as comments in the file. I don't mind the file is large with a lot of comments.
@davidtaikocha could you please add some of the above rationals as comments in the file. I don't mind the file is large with a lot of comments.
Sure, there may be still some small adjustments these days, and i will add more comments in the file after that.
@davidtaikocha Mark this one as draft and let me know when comments are added so we can all take another look.
@davidtaikocha Mark this one as draft and let me know when comments are added so we can all take another look.
Comments added.
And here are some TPS concerns from my side, since both using Bridge and creating a L2 contract are encouraged in Alpha-1, and for Alpha-2, now we set expected block time to 20s in comments:
blockMaxGasLimit
is 6M
).maxBytesPerTxList
is 120K
)