feat(protocol): Additional integration tests, solidity bump, reduce TokenVault contract size

[cyberhorsey]

This adds some additional integration tests, as well as updates Solidity to 0.8.18 so we can take advantage of the named mapping parameters function, which definitely helps readability on some of our more complex mappings, and removes the need for comments. It also fixes the linter/type errors on the new customError handling functions, as well as hardhat-contract-sizer to get to the bottom of the large contract which is failing our CICD randomly, which turned out to be the TokenVault, which is almost at 24.5kb max contract size. I got it down to 22.7kb with custom errors refactoring and slight modifications elsewhere, and now it seems to not fail our CICD.

However, solhint is not ready for named parameter mappings just yet: https://github.com/protofire/solhint/issues/397

I can go ahead and undo those changes if we want to wait for solhint to be ready for us before we do this change.

@dantaik as well, in solidity 0.9.0, block.difficulty will be removed from Solidity, which we use for setting the mixHash when proposing a block.

[dantaik]

I can go ahead and undo those changes if we want to wait for solhint to be ready for us before we do this change. No need to care about the solint errors.

@dantaik as well, in solidity 0.9.0, block.difficulty will be removed from Solidity, which we use for setting the mixHash when proposing a block I'll take a look.

[davidtaikocha]

@dantaik as well, in solidity 0.9.0, block.difficulty will be removed from Solidity, which we use for setting the mixHash when proposing a block.

I believe we should use block.prevrandao instead, which was introduced in solidity 0.8.18:

[davidtaikocha]

And seems there are some errors in github action (test:tokenomics), hmm not sure if its because of the changes in this PR ?

[dantaik]

@cyberhorsey I accidentally merged my PR onto yours before fixing test bugs. Please undo the last PR of mine.

[cyberhorsey]

@cyberhorsey I accidentally merged my PR onto yours before fixing test bugs. Please undo the last PR of mine.

I have already commit on top of it :D. So far it actually seems the tests pass with your PR merged in, it is simply having issues with solidity-coverage library and the new 0.8.18 solidity changes as well. I will look at them tomorrow.

[davidtaikocha]

and looks like abi.encodePacked and virtual modifier will also be removed / disallowed in 0.9.0, seems we will also need to change them in current codebase later (not now maybe).

[dantaik]

and looks like abi.encodePacked and virtual modifier will also be removed / disallowed in 0.9.0, seems we will also need to change them in current codebase later (not now maybe).

Let's fix those later with new solidity features.

[cyberhorsey]

@dantaik @davidtaikocha solidity-coverage library is not ready for new solidity features, and the repo is potentially semi-abandoned (hanging open issues and PRs for quite some time).

I have submitted a PR to fix the repo so it works with 0.8.18: https://github.com/sc-forks/solidity-coverage/pull/783 But I am unsure if it will be merged in. In the meantime, I have changed our dependency to point to my fork. Let me know if that isn't OK with you. I could also fork it under the taikoxyz org, or we could disable coverage.

[codecov[bot]]

Codecov Report

Merging #13155 (f8230f9) into main (4f7ab64) will increase coverage by 0.21%. The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main   #13155      +/-   ##
==========================================
+ Coverage   60.95%   61.16%   +0.21%     
==========================================
  Files         115      115              
  Lines        3391     3394       +3     
  Branches      460      463       +3     
==========================================
+ Hits         2067     2076       +9     
+ Misses       1241     1234       -7     
- Partials       83       84       +1     

Flag	Coverage Δ		*Carryforward flag
bridge-ui	92.61% <ø> (ø)		Carriedforward from f54fa8f
protocol	52.21% <50.00%> (+0.41%)	:arrow_up:
relayer	66.06% <ø> (ø)		Carriedforward from f54fa8f
ui	100.00% <ø> (ø)		Carriedforward from f54fa8f

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

Impacted Files	Coverage Δ
packages/protocol/contracts/L1/TaikoL1.sol	32.69% <ø> (ø)
...ckages/protocol/contracts/L1/libs/LibProposing.sol	6.57% <ø> (+0.16%)	:arrow_up:
packages/protocol/contracts/L1/libs/LibProving.sol	0.00% <0.00%> (ø)
packages/protocol/contracts/L2/TaikoL2.sol	53.84% <ø> (ø)
packages/protocol/contracts/bridge/EtherVault.sol	76.19% <ø> (ø)
...s/protocol/contracts/bridge/libs/LibBridgeData.sol	100.00% <ø> (ø)
...es/protocol/contracts/test/libs/TestLibProving.sol	0.00% <0.00%> (ø)
...s/protocol/contracts/thirdparty/AddressManager.sol	100.00% <ø> (ø)
...protocol/contracts/thirdparty/ERC20Upgradeable.sol	61.44% <ø> (ø)
packages/protocol/contracts/bridge/TokenVault.sol	51.08% <55.55%> (+0.51%)	:arrow_up:
... and 1 more

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

[dantaik]

I could also fork it under the taikoxyz org, or we could disable coverage.

I think we can fork the repo under taiko's account.

[cyberhorsey]

I could also fork it under the taikoxyz org, or we could disable coverage.

I think we can fork the repo under taiko's account.

OK, done