0.38.x: Move (some) signatures and decoders of versioned ABIs to a dedicated location #6569
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
…location (Essentially a cherry-pick from develop, but before that PR got merged to develop. But see PR #6564.) Start of a new method for having one-stop-shopping for everything we need related to system contract ABIs. Signed-off-by: David Bakin <117694041+david-bakin-sl@users.noreply.github.com>
Signed-off-by: David Bakin <117694041+david-bakin-sl@users.noreply.github.com>
…removed Signed-off-by: David Bakin <117694041+david-bakin-sl@users.noreply.github.com>
Nana-EC
approved these changes
May 12, 2023
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## release/0.38 #6569 +/- ##
===================================================
+ Coverage 67.97% 91.00% +23.03%
+ Complexity 22317 17294 -5023
===================================================
Files 2018 1327 -691
Lines 135994 49700 -86294
Branches 7686 4998 -2688
===================================================
- Hits 92437 45230 -47207
+ Misses 42060 3515 -38545
+ Partials 1497 955 -542
☔ View full report in Codecov by Sentry. |
|
SonarCloud Quality Gate failed. |
...ava/com/hedera/node/app/service/mono/store/contracts/precompile/impl/SystemContractAbis.java
Show resolved
Hide resolved
Neeharika-Sompalli
approved these changes
May 12, 2023
nickpoorman
approved these changes
May 12, 2023
Neeharika-Sompalli
deleted the
06559-vs-0.38-refactor-system-contract-signatures
branch
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.
(Essentially a cherry-pick from develop, but done on my machine before the merge to develop happened. See #6564 for the original PR.)
Start of a new method for having one-stop-shopping for everything we
need related to system contract ABIs. (Subsequent additions will
include all system contract ABIs, and then moving to use this enum
as the source of ground truth for ABI (names, selectors, etc.)
everywhere.
N.B.: The initial goal of this is to simplify correct maintenance by putting everything related to our ABI in one place, instead of being scattered all over the place the way it is now. The ultimate goal is to generate this enum from the ground truth Solidity system contracts in our smart contracts repo.
Of the versioned ABIs, in this PR are: burn, mint, update token expiry info.
Subsequent PR will add all the creates, crypto transfer, and update token info.
Signed-off-by: David Bakin 117694041+david-bakin-sl@users.noreply.github.comDescription:
Related issue(s):
Fixes #6559
Notes for reviewer:
The meat is in the new enum
SystemContractAbiswhich provides an enum element for each versioned ABI that supplies things like its selector and its decoder and also other interesting stuff. All the versioned ABIs are in there, and the unit test checks that the selectors computed from the signatures given match the ones we depend on inAbiConstants.The enum contains all versioned ABIs though this PR only has fixed up the code for burn, mint, and update token expiry.
Future work will finish the work on incorporating the versioned ABIs from this enum into the precompiles, add all the non-versioned ABIs, and eventually refactor with an aim of eliminating other files where this stuff is scattered about, including
ParsingConstantsandDecodingFacade. We should also generateAbiConstants.Checklist