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
Fix handling of custom fees in token create transaction builder #9632
Conversation
Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>
Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>
Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #9632 +/- ##
==============================================
+ Coverage 0 65.29% +65.29%
- Complexity 0 29689 +29689
==============================================
Files 0 3260 +3260
Lines 0 124577 +124577
Branches 0 12916 +12916
==============================================
+ Hits 0 81345 +81345
- Misses 0 40158 +40158
- Partials 0 3074 +3074
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not certain I understand the point of the change; can you add a unit test that fails on develop
and passes here to clarify?
Hey @tinker-michaelj I was working on 9398, the tests were failing, producing a SUCCESS status instead of the expected INVALID_CUSTOM_FEE_COLLECTOR. Upon investigating the workflow, I discovered an issue with the way custom fees are added in the transaction builder for the create token operation, which I have addressed in this PR. The if checks in Additionally, the way we were adding the fees was incorrect. The customFees(List) method in TokenCreateTransactionBody simply assigns/overrides the local field for customFees in the TokenCreateTransactionBody. Consequently, the following issue occurred:
This issue became apparent when I noticed that the CustomFees in TokenCreateTransactionBody were always zero, which was due to the absence of royalty fees, resulting in the collection being overridden with an empty list. I have not enabled any tests in this PR since after this fix the tests still fails, but now due to the missing child record logic. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the explanation @MrValioBg !
Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech>
Signed-off-by: Valentin Valkanov <valentin.valkanov@limechain.tech> Signed-off-by: Nick Poorman <nick@swirldslabs.com>
Description:
This PR fixes the builder logic for adding customFees to the TransactionBuilder. As well as improves code readability in CreateSyntheticTxnFactory.
In the TokenCreateWrapper we initialize the
fixedFees
,fractionalFees
&royaltyFees
in the constructor to List.of(); So its not possible for them to be null. Also customFees method inTokenCreateTransactionBody
directly overrides the Collection rather than adding the elements of the newly passed ones to it.This PR addresses those issues.
Checklist