Closed xxAVOGADROxx closed 3 weeks ago
Amazing! Thanks for the fix. Could you add a unit test to cover the new code being added?
@cffls Please let me know if any changes are needed. Thanks!
I had hoped that my changes to the multi asset/asset class would disallow empty/ zero assets (see the normalization methods). I am wondering if there is an oversight in the implementation that introduces this need for a fix inside the tx builder? Do we need to run normalization after init as well?
In fact @xxAVOGADROxx I reverted your changes to the txbuilder class and the test case you added still passes. Please make sure that the test case fails before your applied change, otherwise it is not clear what exactly is the change introduced.
I have enabled workflows for this PR so you can directly see if any added unit test increases coverage.
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 85.22%. Comparing base (
388ab97
) to head (66ef7d5
). Report is 4 commits behind head on chang.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Hello @nielstron, I've added the correct unit test (verifying that it solves the issue). Additionally, I realized the logic for filtering empty/zero assets wasn't necessary, so I've removed that part.
Thanks @xxAVOGADROxx for adding the test. I realized that this error originated from __iadd__
in Asset
and MultiAsset
. In the previous implementation, __iadd__
will only update assets whose value has changed, but it won't update if the key is deleted. I pushed a fix to this behavior and we won't need to change txbuilder now :D
Problem
The logic did not properly handle cases where assets under one policy were burned. This led to incorrect outputs in the
multi_asset
structure, causing transaction outputs to be improperly computed.Solution
A fix was applied to ensure proper handling of burning assets under the same policy.
policy_id_1
) are properly removed from themulti_asset
map.Changes
_calc_change
method oftxbuilder.py
.Additional information
Tested on preproduction.