add tests for more sufficient gas consumption testing in callWithExactGas library #13326
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
RensR
reviewed
May 27, 2024
| "@chainlink/contracts": patch | ||
| --- | ||
|
|
||
| Add Additional tests to the callWithExactGasLibrary to ensure proper gas usage |
There was a problem hiding this comment.
Imo there is no changeset required for changes like these
There was a problem hiding this comment.
I know but the the changeset checker kept causing problems without one.
There was a problem hiding this comment.
You can ignore the failing CI for changeset checker. It's not required
RensR
reviewed
May 27, 2024
contracts/.gas-snapshot
Outdated
| @@ -0,0 +1,52 @@ | |||
| BurnMintERC677_approve:testApproveSuccess() (gas: 55512) | |||
There was a problem hiding this comment.
Please delete, this is not the correct snapshot location. Any time you see an unexpected new file please ensure you intended to create it
|
#13331 fixes CI |
… conflicts but broke the CI
|
RensR
changed the title
RensR
approved these changes
May 28, 2024
matYang
approved these changes
May 29, 2024
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR replaces the one I just closed, #13217 which could not be merged due to an issue with the sigchecker not signing every commit.
The helper library
CallWithExactGas.soldid not have any tests that ensured that all of the gas provided is properly consumed in the event that an incorrect amount of gas is passed in. CCIP makes extensive use of this library and so this tests attempts to ensure that all gas is properly utilized when contracts are called with the helper functions.Added two tests to
callWithExactGas.t.soltest_callWithExactGasAndConsumeAllwhich creates a mock contract and ensures that the amount of gas consumed is the same as the amount passed in.test_callWithExactGasAndConsumeMoreThanProvidedwhich attempts to consume more gas than is provided in the event that gasLimit > (63/64) of available gas passed to the child call in conjunction with EIP-150