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

SubscribeInvoices, can't get settle_index 1 #2469

Closed
ottosuess opened this issue Jan 14, 2019 · 13 comments
Closed

SubscribeInvoices, can't get settle_index 1 #2469

ottosuess opened this issue Jan 14, 2019 · 13 comments
Labels
api gRPC P3 might get fixed, nice to have payments Related to invoices/payments rpc Related to the RPC interface

Comments

@ottosuess
Copy link
Contributor

ottosuess commented Jan 14, 2019

Background

Documentation for the SubscribeInvoices call says about the settle_index:

If specified (non-zero), then we’ll first start by sending out notifications for all settled indexes with an settle_index greater than this value.

That means if I set the settle_index to 0 i only get notified about new Invoices.
If I set the settle_index to 1 the first Invoice I get notified about is the one with an settle_index of 2.

So I'm always missing the first first invoice.

Would be cool if I could just rely on SubscribeInvoices to get notified about all my invoices. But right now I have to use the ListInvoices call to page through all my invoices until the first invoice is settled to make this work.

same for the add_index.

Possible Solutions

  • make settle_index start at 2
  • send out notifications for all indexes with an index greater or equal than the provided value
  • let users of the api set settle_index to 0 and make the default value something else (null or -1)

Your environment

  • lnd version 0.5.1-beta commit=v0.5.1-beta-154-g71444e74acc9896d31205bd16b70fd0f85fc691b
  • Linux btc 4.15.0-42-generic BIP-141 name fix #45-Ubuntu SMP Thu Nov 15 19:32:57 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux
  • Bitcoin Core Daemon version v0.17.0.0-ge1ed37edaedc85b8c3468bd9a726046344036243
@cfromknecht
Copy link
Contributor

@ottosuess have you been able to reproduce this on your node? I think it may just be a documentation issue, as from my understanding, we send out all indexes greater than or equal to the specified index

@ottosuess
Copy link
Contributor Author

ottosuess commented Jan 15, 2019

@cfromknecht yes, when I set settleIndex = 1, the first invoice I get is the one with settleIndex == 2.

@halseth
Copy link
Contributor

halseth commented Jan 16, 2019

Could we add new parameters to the API that uses greater or equal, then deprecate the old ones?

@riperk
Copy link

riperk commented Jul 13, 2019

What about treating 0 differently from undefined? i.e. If you only want new settlements, you don't set a settle_index. If you set a settle_index to value (including 0) then you get all above that value

@halseth
Copy link
Contributor

halseth commented Jul 29, 2019

@riperk I think the default value is 0, so you cannot differentiate that from not set.

@Roasbeef Roasbeef added api gRPC P3 might get fixed, nice to have payments Related to invoices/payments rpc Related to the RPC interface labels Oct 25, 2019
@instagibbs
Copy link

Any update on this wrinkle? New hold invoice system avoids this issue but curious to see what the cleanest fix is.

@Roasbeef
Copy link
Member

Roasbeef commented Jun 4, 2020

We're planning a new sub-server RPC call to fix all the issues with the existing base service invoice subscription endpoint. Until then, the hacky way would be to use LIstInvoices to just grab that first invoice (if you really need it), then you subscription for the rest. I'd imagine an application is storing the two indexes (settle and add) on their end so they can resume consuming the stream after a restart.

@instagibbs
Copy link

instagibbs commented Jun 4, 2020 via email

@tim2CF
Copy link

tim2CF commented Aug 17, 2020

@riperk I think the default value is 0, so you cannot differentiate that from not set.

Unfortunately protobuf standard don't support generics, so we can't just define Maybe a generic type in native terms of protobuf. This will not compile:

message Nothing {

}

message Maybe a {
  oneof unMaybe {
    a just = 1;
    Nothing nothing = 2;
  }
}

So absence representation is not possible in general, but it's still might be done this way:

https://github.com/protocolbuffers/protobuf/blob/d61aede89cf188367766b971f59cf57d7835d8e8/src/google/protobuf/wrappers.proto#L31-L39

The same practice can be used to represent absence of value of any custom enum type as well. These small messages-wrappers are a bit similar to newtype expressions in Haskell or Rust.

@juansoft-spain
Copy link

juansoft-spain commented Jun 17, 2022

Hi! Any change in this issue? I noticed that REST API has the same problem. Example:

curl -X GET --cacert tls.cert --header "$MACAROON_HEADER" https://localhost:8081/v1/invoices/subscribe\?settle_index\=1

Returns a collection of invoices that always have the first settle_index == 2. If the value of settle_index query param is set to 0, only new invoices will be notified.

Is the workaround still been the suggested by @Roasbeef?

Thank you very much in advance!

@guggero
Copy link
Collaborator

guggero commented Jun 17, 2022

REST is just a generic wrapper around the gRPC API, so all features (and problems) are the same.
Unfortunately that particular issue hasn't been addressed yet and the proposed workaround is still required.

@juansoft-spain
Copy link

REST is just a generic wrapper around the gRPC API, so all features (and problems) are the same. Unfortunately that particular issue hasn't been addressed yet and the proposed workaround is still required.

Many thanks for your fast reply @guggero and thanks also for your work in this project. So, I will use the workaround!

@Roasbeef
Copy link
Member

Closing this as we'll be able to resolve it with this larger overhaul (this is tech debt): #6288

The work around is to fetch the first invoice, then subscribe for the rest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api gRPC P3 might get fixed, nice to have payments Related to invoices/payments rpc Related to the RPC interface
Projects
None yet
Development

No branches or pull requests

9 participants