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

Updated TypeReference object to support nested types to support truly… #2023

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

calmacfadden
Copy link

What does this PR do?

This PR adds support for decoding dynamic structs without the need for a class extending DynamicStructs to be added at compile time.

I have extended the TypeReference class to have nested innerType's which means you can represent a Struct using TypeReference.

Where should the reviewer start?

  • Look at the TypeReference.java changes to see the innerTypes which have been added.
  • Check the decodeDynamicStruct function in TypeDecoder.java. I have kept the current decoding method intact but renamed it decodeDynamicStructElementsWithCtor and created a new function called decodeDynamicStructElementsWithInnerTypeRefs which is a duplicate of the original function but it has been modified to support the nested type refereces.
  • Check the unit test testDynamicStructFix() in the DefaultFunctionEncoderTest.java which demonstrates how it is expected to be used.

Why is it needed?

There needs to be a way to decode dynamic structs without having to create a corresponding class in the project. This allows for the ability to interact with any contract dynamically based on user inputs.

Checklist

  • I've read the contribution guidelines.
  • I've added tests (if applicable).
  • I've added a changelog entry if necessary.

@calmacfadden calmacfadden marked this pull request as draft March 26, 2024 13:39
Copy link

codecov bot commented Apr 2, 2024

Codecov Report

Attention: Patch coverage is 86.46617% with 18 lines in your changes are missing coverage. Please review.

Project coverage is 69.31%. Comparing base (ccfef8b) to head (cff3845).
Report is 57 commits behind head on master.

Files Patch % Lines
abi/src/main/java/org/web3j/abi/TypeDecoder.java 88.70% 12 Missing and 2 partials ⚠️
abi/src/main/java/org/web3j/abi/TypeReference.java 50.00% 4 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #2023      +/-   ##
============================================
+ Coverage     69.22%   69.31%   +0.08%     
- Complexity     3117     3143      +26     
============================================
  Files           493      493              
  Lines         13090    13230     +140     
  Branches       1692     1710      +18     
============================================
+ Hits           9062     9170     +108     
- Misses         3537     3569      +32     
  Partials        491      491              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gtebrean
Copy link
Contributor

gtebrean commented Apr 2, 2024

Great work! I've bee trough the changes and this new approach with extension of the TypeReference is easy to follow.
Also I can see that the checks are passing.
There might be some small comments in terms of code structure but we keep that for later.

Will be helpful if you can add some more test for testing of decoding and encoding of the new struct type reference. Ideally to cover all the existing test cases for DynamicStruct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants