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

Fix: negative minted amount is also requested #264

Merged
merged 7 commits into from
Oct 22, 2023

Conversation

nielstron
Copy link
Contributor

@nielstron nielstron commented Jul 20, 2023

For transactions that only specified a token to be burned it could currently happen that the token was not selected by the input selector - because negative minting amounts were not counted as requested by the tx builder. This is a bugfix.

  • Add a unit test

@nielstron nielstron force-pushed the fix/minted_amount_requested branch from 4f667ff to b531e56 Compare July 20, 2023 16:22
@nielstron nielstron force-pushed the fix/minted_amount_requested branch from b531e56 to bfa1773 Compare July 20, 2023 16:25
@nielstron
Copy link
Contributor Author

The only error appears to have been uploading to Codecov

@codecov-commenter
Copy link

codecov-commenter commented Jul 21, 2023

Codecov Report

Merging #264 (bde51ea) into main (cd975db) will increase coverage by 0.02%.
Report is 1 commits behind head on main.
The diff coverage is 91.66%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##             main     #264      +/-   ##
==========================================
+ Coverage   84.73%   84.75%   +0.02%     
==========================================
  Files          26       26              
  Lines        3033     3044      +11     
  Branches      731      740       +9     
==========================================
+ Hits         2570     2580      +10     
  Misses        348      348              
- Partials      115      116       +1     
Files Coverage Δ
pycardano/txbuilder.py 90.54% <91.66%> (+<0.01%) ⬆️

@nielstron
Copy link
Contributor Author

Note: this breaks if the burned amount is manually added as input, because the minted amount is added to the selected_amount, resulting in 0. This is valid as the amount can not be used anymore later. However modeling it as a requested output leverages the coinselection to automatically fetch the required input for the burning.

@cffls
Copy link
Collaborator

cffls commented Jul 22, 2023

Nice! I will try to add a test. Btw, is there a way to allow me to push to your branch (OpShin:fix/minted_amount_requested)?

@nielstron
Copy link
Contributor Author

You should have access now :)

@nielstron
Copy link
Contributor Author

I fixed the previously mentioned issue when pre-selected inputs contain the token to be burned. What's missing is a unit test that checks the automatic selection of tokens to be burned

@cffls cffls merged commit 37f1b3b into Python-Cardano:main Oct 22, 2023
22 checks passed
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.

3 participants