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

Add additional solidity tests #14979

Closed
2 of 9 tasks
dantaik opened this issue Oct 18, 2023 · 3 comments
Closed
2 of 9 tasks

Add additional solidity tests #14979

dantaik opened this issue Oct 18, 2023 · 3 comments

Comments

@dantaik
Copy link
Contributor

dantaik commented Oct 18, 2023

Describe the feature request

The test coverage report generated by pnpm test:coverage shows a lack of test coverage. (TaikoL1's coverage is almost zero as foundry doesn't support Library test coverage, we need to figure out the actual coverage later).

The test coverage report indicates we need to add more tests to test the following:

High priority

  • SignalService.sol: the proveSignalReceived function needs more tests to coverage the for-loop. This will coverage the verifyInclusionProof function in LibSecureMerkleTrie.sol.
  • TaikoL2.sol: the _rewardParentBlock function should be tested more thoroughly. There are quite a few if-else statements. Same with the _calc1559BaseFee function which will also coverage Lib1559.sol -- the file's coverage is also zero.
  • BridgedERC1155.sol and BridgedERC721.solhas no coverage. BridgedERC20.sol also has no coverage, which is surprising.
  • XxxVault.sol: all vaults also has no coverage, this is also surprising.
  • Bridge.sol: should add a test for recallMessage; we should also add a test to send a message whose to value is 0x0 or the bridge address on the destination chain.

Low priority

  • In many tests, the init functions are not covered. Need to figure out what caused it. TaikoToken.sol, and TaikoL2.sol are among those files.
  • GuardianProver.sol has zero test coverage.
  • TaikoA6TierProvider.sol has zero test coverage, we should add a very simple test for it.
  • L1/verifiers/Xxx.sol: all verifiers have zero test coverage.
@adaki2004
Copy link
Contributor

As per discussed, most of the 0% coverage is due to forge coverage issues. Will try to incorporate more tests tho, will check manually each what could be missing.

Copy link
Contributor

This issue is stale because it has been open for 30 days with no activity.

Copy link
Contributor

This issue was closed because it has been inactive for a week since being marked as stale.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale May 21, 2024
@github-project-automation github-project-automation bot moved this from 📝 Todo to ✅ Done in Taiko Project Board May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

2 participants