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

refactor(protocol): various changes based on Brecht & Dani's feedback #13427

Merged
merged 24 commits into from
Mar 24, 2023

Conversation

dantaik
Copy link
Contributor

@dantaik dantaik commented Mar 23, 2023

Previously

test2/TaikoL1.t.sol:TaikoL1WithConfig contract
Function Name min avg median max # calls
proposeBlock 18560 24663 18580 65707 339
proveBlock 56069 64958 56069 132369 339
verifyBlocks 17191 41931 17191 148496 331

Now

test2/TaikoL1.t.sol:TaikoL1WithConfig contract
Function Name min avg median max # calls
proposeBlock 19057 29415 19077 88104 309
proveBlock 55834 65888 55834 132234 309
verifyBlocks 14944 16139 15869 79299 301

@codecov
Copy link

codecov bot commented Mar 23, 2023

Codecov Report

Merging #13427 (18c915b) into major_protocol_upgrade_rebase (4ddbd9b) will decrease coverage by 0.26%.
The diff coverage is 0.00%.

❗ Current head 18c915b differs from pull request most recent head 975a53d. Consider uploading reports for the commit 975a53d to get more accurate results

@@                        Coverage Diff                        @@
##           major_protocol_upgrade_rebase   #13427      +/-   ##
=================================================================
- Coverage                          40.07%   39.81%   -0.26%     
=================================================================
  Files                                112      112              
  Lines                               3404     3403       -1     
  Branches                             413      405       -8     
=================================================================
- Hits                                1364     1355       -9     
- Misses                              1952     1960       +8     
  Partials                              88       88              
Flag Coverage Δ *Carryforward flag
bridge-ui 94.22% <ø> (-0.44%) ⬇️ Carriedforward from a9365be
protocol 0.00% <0.00%> (ø)
relayer 62.30% <ø> (-0.15%) ⬇️ Carriedforward from a9365be
ui 100.00% <ø> (ø) Carriedforward from a9365be

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
packages/protocol/contracts/L1/TaikoConfig.sol 0.00% <ø> (ø)
packages/protocol/contracts/L1/TaikoL1.sol 0.00% <0.00%> (ø)
...ckages/protocol/contracts/L1/libs/LibProposing.sol 0.00% <0.00%> (ø)
packages/protocol/contracts/L1/libs/LibProving.sol 0.00% <0.00%> (ø)
...kages/protocol/contracts/L1/libs/LibTokenomics.sol 0.00% <ø> (ø)
packages/protocol/contracts/L1/libs/LibUtils.sol 0.00% <0.00%> (ø)
...ckages/protocol/contracts/L1/libs/LibVerifying.sol 0.00% <0.00%> (ø)
...ackages/protocol/contracts/test/L1/TestTaikoL1.sol 0.00% <0.00%> (ø)
.../contracts/test/L1/TestTaikoL1EnableTokenomics.sol 0.00% <0.00%> (ø)

... and 11 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Base automatically changed from fix_comment2 to major_protocol_upgrade_rebase March 23, 2023 12:26
@dantaik dantaik changed the title fix comment refactor(protocol): improve storage based on review feedback Mar 23, 2023
@dantaik dantaik marked this pull request as ready for review March 23, 2023 13:42
@dantaik dantaik changed the title refactor(protocol): improve storage based on review feedback refactor(protocol): various changes based on Brecht & Dani's feedback Mar 23, 2023
uint256 fcId = state.forkChoiceIds[id][parentHash];
if (blk.blockId != blockId) revert L1_BLOCK_ID();

uint256 fcId = state.forkChoiceIds[blockId][parentHash];
if (fcId == 0 || fcId >= blk.nextForkChoiceId)
Copy link
Contributor

Choose a reason for hiding this comment

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

I see it as above, dont know in which scenario it could happen.
If fcId == 0, then blk.nextForkChoiceId == 1 -> No proof submitted.
If blk.nextForkChoiceId == 2 a proof is submitted either by an oracle or single prover, but in such case the second proof submitted (when unverified) does not increase blk.nextForkChoiceId above 2.

Copy link
Contributor Author

@dantaik dantaik Mar 24, 2023

Choose a reason for hiding this comment

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

  • fcId==0 means the fork choice does not exist
  • fcId >= blk.nextForkChoiceId shall never happen. I convert it into an assert

@dantaik dantaik merged commit 0b8f7fe into major_protocol_upgrade_rebase Mar 24, 2023
@dantaik dantaik deleted the fix_comment3 branch March 24, 2023 16:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants