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

Expose listoffer and offer over grpc #6896

Merged
merged 8 commits into from Feb 7, 2024

Conversation

hydra-yse
Copy link

Resolves #6892

I was unsure of some of the protobuf definitions for the recurrence-related types, had to take a look at bolt12.c and see from there. Definitely requires a second look.

Currently missing:

  • Regenerating .msggen.json based on doc schemas (the msggen script does not seem to recognize the rootdir positional argument after being installed and sourced, will check)

@cdecker cdecker self-assigned this Dec 4, 2023
@cdecker cdecker added this to the v24.02 milestone Dec 4, 2023
@cdecker cdecker force-pushed the 20231122-offer-grpc branch 2 times, most recently from 09423a5 to b772640 Compare December 21, 2023 14:03
@cdecker
Copy link
Member

cdecker commented Dec 21, 2023

Looks like the "flexibility" of having multiple different ways to specify a recurrence is biting us here. We should really consolidate on one way to express things, and deprecate the alternative uses. This ought to simplify the schema, as well as supporting the functionality going forwards. WDYT @rustyrussell? Ideally I'd like to just use the string variant, as that seems to be expressive enough.

@cdecker cdecker changed the title 20231122 offer grpc Expose listoffer and offer over grpc Dec 22, 2023
@rustyrussell
Copy link
Contributor

See #7034

@cdecker
Copy link
Member

cdecker commented Feb 4, 2024

Rebased on top of #7034 to fold the simplification of arguments in

vacwmX and others added 8 commits February 7, 2024 17:44
The `amount` field is intended to be either a native unit (`msat`,
`sat` or `btc`) or it can also be a non-native unit that needs to be
converted into native when creating the invoice. As such limiting the
`amount` to only be native is very restrictive.
I had to do some trickery with the interchangeability between string
and int, but now it works as expected.
@cdecker
Copy link
Member

cdecker commented Feb 7, 2024

The schema had a minor issue with the stringified variants and the object variants. The object variant isnt accessible from the RPC, rather we must use the string variant. This now passes the tests.

Also rebased on top of master

@cdecker cdecker merged commit 99aa1e8 into ElementsProject:master Feb 7, 2024
36 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.

grpc: adding support for offer command
3 participants