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

Implement spending of entire UTxOs using build command with no change outputs #3188

Merged
merged 1 commit into from
Sep 13, 2021

Conversation

Jimbo4350
Copy link
Contributor

Resolves #3041

@Jimbo4350 Jimbo4350 changed the title Implement spending of entire utxos with the transaction build command Implement spending of entire UTxOs using build command with no change outputs Sep 10, 2021
@Jimbo4350 Jimbo4350 force-pushed the jordan/fix-zero-change-transaction-build branch from a1573cc to 9b9b127 Compare September 10, 2021 18:27
Copy link
Contributor

@dcoutts dcoutts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this actually work? Doesn't the balance check already fail if we have a balance that is less than the min utxo (which of course includes zero).

Note also that even if this passes the balance check, it does not involve paying any less in fees for the case where the change comes out to be zero. We're still calculating the fees assuming we'll have a change output. That's a much harder one to solve (and why I didn't bother in the first place).

@Jimbo4350 Jimbo4350 force-pushed the jordan/fix-zero-change-transaction-build branch from 9b9b127 to f7b83ec Compare September 13, 2021 14:03
@Jimbo4350
Copy link
Contributor Author

Jimbo4350 commented Sep 13, 2021

Does this actually work?

I forgot to push a local change that modifies balanceCheck so this works now. I'm not going to bother to go through the hassle of reducing the fee calculation because I agree it's not worth it.

Copy link
Contributor

@dcoutts dcoutts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, though as suggested below I think it's worth explaining properly in the code comments.

cardano-api/src/Cardano/Api/Fees.hs Outdated Show resolved Hide resolved
@Jimbo4350 Jimbo4350 force-pushed the jordan/fix-zero-change-transaction-build branch from f7b83ec to 3814f02 Compare September 13, 2021 14:53
@Jimbo4350
Copy link
Contributor Author

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Sep 13, 2021

Build succeeded:

@iohk-bors iohk-bors bot merged commit e2c428b into master Sep 13, 2021
@iohk-bors iohk-bors bot deleted the jordan/fix-zero-change-transaction-build branch September 13, 2021 15:19
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.

[BUG] - transaction build attempts to create UTxO with 0 lovelace as change
2 participants