Fixed transaction imbalance when burning assets from the same policy

[xxAVOGADROxx]

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.

• Unite test:
  Assets that are burned under a given policy ID (policy_id_1) are properly removed from the multi_asset map.

Changes

• In the _calc_change method of txbuilder.py.

Additional information

Tested on preproduction.

[cffls]

Amazing! Thanks for the fix. Could you add a unit test to cover the new code being added?

[xxAVOGADROxx]

@cffls Please let me know if any changes are needed. Thanks!

[nielstron]

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?

[nielstron]

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.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.22%. Comparing base (388ab97) to head (66ef7d5).
Report is 4 commits behind head on chang.

Additional details and impacted files

@@            Coverage Diff             @@
##            chang     #376      +/-   ##
==========================================
- Coverage   85.24%   85.22%   -0.02%     
==========================================
  Files          32       32              
  Lines        4209     4211       +2     
  Branches     1059     1059              
==========================================
+ Hits         3588     3589       +1     
  Misses        435      435              
- Partials      186      187       +1     

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

[xxAVOGADROxx]

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.

[cffls]

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