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

feat: chain swaps cli #136

Merged
merged 25 commits into from
Apr 26, 2024
Merged

feat: chain swaps cli #136

merged 25 commits into from
Apr 26, 2024

Conversation

jackstar12
Copy link
Member

@jackstar12 jackstar12 commented Apr 18, 2024

No description provided.

Copy link
Member

@kilrau kilrau left a comment

Choose a reason for hiding this comment

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

createswap & createchainswap parity:

  • to be in line with behavior of createchainswap, createswap should also print help output instead of starting an "any amount" swap process when called without args. It even says --any-amount is (default: false) but well that's not true
  • boltzcli createswap --any-amount defaults to mainchain, the --currency arg should NOT be optional and print help output. If you absolutely want to default, default to Liquid and remove the --liquid flag.
  • I don't get why createswap has --internal-send AND --wallet. --internal-send sounds very vague and confusing so let's get rid of it and stick with --from-wallet & --to-wallet everywhere, just like in createchainswap. It's desirable that the user specifies which wallet to use and not automagically choose.
  • createchainswap should have the --any-amount feature too
  • There should be examples in both help outputs for --any-amount
  • I like --to-external better to be in line with --from-external. Help output tells me that one has to be a currency, one an address

Adding more later

@jackstar12
Copy link
Member Author

[ ] to be in line with behavior of createchainswap, createswap should also print help output instead of starting an "any amount" swap process when called without args. It even says --any-amount is (default: false) but well that's not true

This isnt supported by the backend yet.

[ ] I don't get why createswap has --internal-send AND --wallet. --internal-send sounds very vague and confusing so let's get rid of it and stick with --from-wallet & --to-wallet everywhere, just like in createchainswap. It's desirable that the user specifies which wallet to use and not automagically choose.

internal-send is currently useful for clients who dont care about any wallet stuff, e.g. RTL.
But it should be external-pay in the cli for consistency.

@kilrau kilrau self-requested a review April 25, 2024 17:02
Copy link
Member

@kilrau kilrau left a comment

Choose a reason for hiding this comment

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

Looking good

boltz/liquid.go Show resolved Hide resolved
cmd/boltzcli/commands.go Outdated Show resolved Hide resolved
@jackstar12 jackstar12 force-pushed the chain-swap-cli branch 2 times, most recently from 6aa7255 to ba60a73 Compare April 26, 2024 19:10
@jackstar12 jackstar12 merged commit eaa78bc into chain-swaps Apr 26, 2024
1 check failed
jackstar12 added a commit that referenced this pull request Apr 27, 2024
* feat: initial chainswaps

* chore: cleanup

* refactor: unify fee and transaction logic

* refactor: use IN when querying refundable swaps

* refactor: use getters and setters for refund and claim pubkey in swaptree

* chore: api naming

* chore: cleanup

* feat: improve wallet logic when creating chain swap

* feat: manual chain swap refunds

* feat: include chainswaps in `ListSwaps` rpc

* chore: consistent naming

* Update nursery/refund.go

Co-authored-by: michael1011 <me@michael1011.at>

* chore: improve proto definitions

* refactor: dont set externalPay implicitly

* chore: cleanup

* refactor: use wallet ids in create request

* feat: only query coop refundable swaps in specific states

* fix: check if swap already has been paid in nursery

* chore: improve error messages

* feat: chain swaps cli (#136)

* feat: chain swap cli

* fix: chain swap help description

* fix: help output missing , statement

* fix: chain swap help description from/to

* fix: createswap,createreverseswap,createchainswap help description

* fix: autoswapper -> autoswap in help outputs, comments

* fix: typo

* refactor: dont implicitly select wallets

* fix: check for state in timeout condition when querying refundable swaps

* fix: only set fees of valid outputs after transaction was created

* feat: implicitly set external pay in cli when not specifying from wallet

* chore: cleanup

* fix: check if value is less than fee when constructing transaction

* feat: return output specific errors when creating transactions

* fix: refund help description

* fix: --from-external, --to-address help output

* refactor: dont default to mainchain when creating swap

* refactor: use from and to wallet terminology everywhere

* chore: add db migration

* fix: display correct refund transaction when refunding in cli

* fix: show correct timeout in cli

* feat: recover pending chain swaps

* fix: correctly set external pay when creating submarine swap

* feat: display currency of lockup tx in cli

* fix: dont try to broadcast empty transaction

---------

Co-authored-by: Kilian <19181985+kilrau@users.noreply.github.com>

* docs: improve grpc comments

* fix: transaction reconstruction logic

* ci: temporarily use chainswaps branch of regtest

* text: wait with mining blocks

* test: remove flaky batch test for now

can be added back later when batching has been refactored

---------

Co-authored-by: michael1011 <me@michael1011.at>
Co-authored-by: Kilian <19181985+kilrau@users.noreply.github.com>
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.

None yet

3 participants