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: get swap by payment hash #377

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jackstar12
Copy link
Member

@jackstar12 jackstar12 commented Jan 20, 2025

todo:

  • db migration
  • tests

pkg/boltzrpc/boltzrpc.proto Outdated Show resolved Hide resolved
@@ -500,6 +500,8 @@ message ClaimSwapsResponse {

message GetSwapInfoRequest {
string id = 1;
Copy link
Member

Choose a reason for hiding this comment

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

This should be optional too?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried to, but getting this error:

➜  boltz-client git:(feat/get-swap-by-payment-hash) ✗ make proto
 Generating protosbufs
eval cd pkg/boltzrpc && ./gen_protos.sh
--grpc-gateway_out: optional field not allowed in field path: id in id

Some naming issue with our grpc gateway solution. So we cant change the without changing the name which would be breaking for our rest wrappers

Comment on lines 502 to 504
string id = 1;
// only implemented for submarine swaps for now
optional bytes payment_hash = 2;
Copy link
Member

Choose a reason for hiding this comment

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

How about a oneof?

Copy link
Member Author

Choose a reason for hiding this comment

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

breaking change, unless you want to add it and deprecate the old id

@jackstar12 jackstar12 force-pushed the feat/get-swap-by-payment-hash branch from 2683e26 to fbac504 Compare January 20, 2025 21:12
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.

2 participants