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

Use the --fee-rate when sending an amount #1922

Merged
merged 12 commits into from
Aug 17, 2023

Conversation

gmart7t2
Copy link
Contributor

@gmart7t2 gmart7t2 commented Mar 12, 2023

"ord wallet send" requires the --fee-rate flag but ignores it when sending cardinals. This PR fixes that and also adds 2 new Outgoing types: 'all', and 'max':

  • 'wallet send all' sends all available cardinals

  • 'wallet send max' maximizes the output by sending all cardinals that are worth more than they cost in fees to send

Users want to be able to consolidate all their cardinals into a single utxo. These new modes make that easier.

Closes #1842 but there is still the issue that "wallet send" leaves the user's inscriptions locked, and fails to run a 2nd time because it fails when it tries to lock the already-locked inscriptions.

Greg Martin added 5 commits March 12, 2023 21:20
…ll', and 'max'.

  * 'wallet send all' sends all available cardinals

  * 'wallet send max' maximizes the output by sending all cardinals that are worth more than they cost in fees to send
@BTCAlchemist
Copy link

I think the workaround until this is merged is to set the default fee-rate in Bitcoin Core and to manually unlock UTXOs between 'ord wallet send' commands. A fix would be helpful since users may not know the workaround and to save time for those who do. @raphjaph Do you have a moment to review this?

@gmart7t2 gmart7t2 force-pushed the improve-sending-cardinals branch 3 times, most recently from 648fc76 to 0fd0b29 Compare April 14, 2023 22:02
Copy link
Collaborator

@raphjaph raphjaph left a comment

Choose a reason for hiding this comment

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

Definitely still needs a test as well.

src/outgoing.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@raphjaph raphjaph left a comment

Choose a reason for hiding this comment

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

Wrote a test and removed the max and all things because I want to implement a proper sweep command that can sweep all inscriptions and cardinals or seperate.

@raphjaph raphjaph enabled auto-merge (squash) August 17, 2023 14:50
@raphjaph raphjaph merged commit 462f38b into ordinals:master Aug 17, 2023
6 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.

sending cardinals requires but ignores --fee-rate
3 participants