Updated TypeReference object to support nested types to support truly…
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.
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 :warning: |
| abi/src/main/java/org/web3j/abi/TypeReference.java | 50.00% | 4 Missing :warning: |
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
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
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
This seems like the missing piece for decoding event data without having to delcare in-Java classes apriori. Would be great if something like this was incorporated.
Can we push this in?
@calmacfadden do you plan to continue the development on this?
@calmacfadden do you plan to continue the development on this?
He has completed his contract at Quant now and therefore this github account is no longer active. I'm unsure if he will pick up this work privately, I would assume not.
So can we either merge it as is or find someone else to complete it? We currently don't have any other resource for this
@calmacfadden do you plan to continue the development on this?
He has completed his contract at Quant now and therefore this github account is no longer active. I'm unsure if he will pick up this work privately, I would assume not.
So can we either merge it as is or find someone else to complete it? We currently don't have any other resource for this
I ended up implementing a version of this PR in my own work for decoding structs. Should I tack it on to this PR? Or start a new PR with the modifications?
@calmacfadden do you plan to continue the development on this?
He has completed his contract at Quant now and therefore this github account is no longer active. I'm unsure if he will pick up this work privately, I would assume not. So can we either merge it as is or find someone else to complete it? We currently don't have any other resource for this
I ended up implementing a version of this PR in my own work for decoding structs. Should I tack it on to this PR? Or start a new PR with the modifications?
@Antlion12 please go for it and if it is possible to track it in this PR will be great.
@Antlion12 please go for it and if it is possible to track it in this PR will be great.
I tried to push my changes to calmacfadden:TrulyDynamicStructs, but I don't have permission to push to that branch. I may have to fork this again.
I've created a fork of this in PR 2076. https://github.com/hyperledger/web3j/pull/2076
Closing this outdated PR, as new PR created by @Antlion12 - #2076