test: add ACP-224 fee manager simulated backend tests#5220
Open
JonathanOppenheimer wants to merge 6 commits intoJonathanOppenheimer/acp224-add-precompilefrom
Open
test: add ACP-224 fee manager simulated backend tests#5220JonathanOppenheimer wants to merge 6 commits intoJonathanOppenheimer/acp224-add-precompilefrom
JonathanOppenheimer wants to merge 6 commits intoJonathanOppenheimer/acp224-add-precompilefrom
Conversation
| errTargetGasTooLowACP224 = errors.New("targetGas must be at least MinTargetGasACP224") | ||
| errTimeToDoubleTooLow = errors.New("timeToDouble must be greater than 0") | ||
| errTimeToDoubleMustBeZero = errors.New("timeToDouble must be 0 when staticPricing is true") | ||
| ErrTargetGasMustBeZero = errors.New("targetGas must be 0 when validatorTargetGas is true") |
Contributor
Author
There was a problem hiding this comment.
We export these errors for use in the test.
JonathanOppenheimer
force-pushed
the
JonathanOppenheimer/acp-224-add-bindings-test
branch
from
bc39c9d to
ed56a19
Compare
| params.RegisterExtras() | ||
| // TODO(JonathanOppenheimer): Remove manual registration when the | ||
| // module is registered unconditionally in init(). | ||
| if err := modules.RegisterModule(acp224feemanager.Module); err != nil { |
Contributor
Author
There was a problem hiding this comment.
The converse if true didn't really make sense here. I think a TODO is okay?
JonathanOppenheimer
added
the
DO NOT MERGE
Contributor
Author
|
Waiting on a clarification before proceeding here. |
…nOppenheimer/acp-224-add-bindings-test
| // | ||
| // catching any disagreement between Go's ABI package and solc's ABI | ||
| // implementation. Note that some tests overlap with contract_test.go (e.g. fee config | ||
| // validation) but the overlap is intentional: contract_test.go tests precompile |
Contributor
Author
There was a problem hiding this comment.
I know the test coverage double dips to some extent.
…nOppenheimer/acp-224-add-bindings-test
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Why this should be merged
This PR addsthe simulated backend tests for the ACP-224 fee manager precompile, exercising the cross-implementation ABI path through a deployed
ACP224FeeManagerTest.solwrapper contract:Go
abi.Pack()-> wrapper (solc-compiled) -> ABI re-encoding -> precompile -> Goabi.Unpack()This catches any disagreement between Go's abi package and solc's ABI implementation. Without the wrapper, both sides of the encoding use Go's abi package, so a bug would be invisible. The wrapper forces solc's compiled bytecode between Go's encoder and decoder, making this a cross-implementation integration test with the EVM interpreter.
How this works
ACP224FeeManagerTest.solon a simulated backend and enables it on the precompile's allowlistsetFeeConfig,getFeeConfig,getFeeConfigLastChangedAt) are routed through the wrapperIACP224FeeManagerinterface binding directly against the precompile addressHow this was tested
Run all the tests in
simulated_test.goNeed to be documented in RELEASES.md?
No