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

Add listreceivedpayments RPC call #2607

Merged
merged 7 commits into from
Apr 4, 2023
Merged

Conversation

rorp
Copy link
Contributor

@rorp rorp commented Mar 3, 2023

No description provided.

@t-bast
Copy link
Member

t-bast commented Mar 17, 2023

Thanks, I think this is useful! A couple high-level comments:

  • I don't think exposing listexpiredinvoices to the API is useful (especially since those invoices get automatically deleted from the DB at regular intervals), why did you add it?
  • I think listpaidinvoices would make more sense if it were called listreceivedpayments and if it returned a Seq[IncomingPayment] instead of just the Invoice, that seems to be the desired feature, isn't it?

@rorp
Copy link
Contributor Author

rorp commented Mar 26, 2023

  • I don't think exposing listexpiredinvoices to the API is useful (especially since those invoices get automatically deleted from the DB at regular intervals), why did you add it?

It can be useful for accounting. Also, automatic purges can be disabled, and when enabled they happen in specific intervals (24h by default).

  • I think listpaidinvoices would make more sense if it were called listreceivedpayments and if it returned a Seq[IncomingPayment] instead of just the Invoice, that seems to be the desired feature, isn't it?

Good idea!

@codecov-commenter
Copy link

Codecov Report

Merging #2607 (eb40988) into master (732eb31) will increase coverage by 0.03%.
The diff coverage is 52.94%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master    #2607      +/-   ##
==========================================
+ Coverage   85.67%   85.71%   +0.03%     
==========================================
  Files         212      212              
  Lines       16962    16966       +4     
  Branches      723      725       +2     
==========================================
+ Hits        14532    14542      +10     
+ Misses       2430     2424       -6     
Impacted Files Coverage Δ
...r-core/src/main/scala/fr/acinq/eclair/Eclair.scala 55.33% <0.00%> (-1.10%) ⬇️
.../main/scala/fr/acinq/eclair/db/DualDatabases.scala 11.11% <0.00%> (ø)
...src/main/scala/fr/acinq/eclair/db/PaymentsDb.scala 85.00% <ø> (ø)
...ain/scala/fr/acinq/eclair/db/pg/PgPaymentsDb.scala 98.60% <100.00%> (ø)
...a/fr/acinq/eclair/db/sqlite/SqlitePaymentsDb.scala 98.65% <100.00%> (ø)
...r/acinq/eclair/payment/receive/InvoicePurger.scala 100.00% <100.00%> (ø)

... and 6 files with indirect coverage changes

Copy link
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

We shouldn't add listexpiredinvoices, but apart from that it looks good to me!

@rorp rorp changed the title listpaidinvoices and listexpiredinvoices RPC calls listpaidinvoices RPC call Apr 2, 2023
t-bast
t-bast previously approved these changes Apr 3, 2023
Copy link
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@t-bast t-bast changed the title listpaidinvoices RPC call Add listreceivedpayments RPC call Apr 3, 2023
@t-bast
Copy link
Member

t-bast commented Apr 3, 2023

Damn, there is a (trivial) conflict in the payment tests, can you rebase on master?

Copy link
Member

@t-bast t-bast left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@t-bast t-bast merged commit e383d81 into ACINQ:master Apr 4, 2023
@rorp rorp deleted the list_paid_invoices branch April 4, 2023 17:50
t-bast added a commit that referenced this pull request Jun 16, 2023
This release introduces a few API changes:

- `audit` now accepts `--count` and `--skip` parameters to limit the
  number of retrieved items (#2474, #2487)
- `sendtoroute` removes the `--trampolineNodes` argument and
  implicitly uses a single trampoline hop (#2480)
- `sendtoroute` now accept `--maxFeeMsat` to specify an upper bound
  of fees (#2626)
- `payinvoice` always returns the payment result when used with
  `--blocking`, even when using MPP (#2525)
- `node` returns high-level information about a remote node (#2568)
- `channel-created` is a new websocket event that is published when
  a channel's funding transaction has been broadcast (#2567)
- `channel-opened` websocket event was updated to contain the final
  `channel_id` and be published when a channel is ready to process
  payments (#2567)
- `getsentinfo` can now be used with `--offer` to list payments sent
  to a specific offer
- `listreceivedpayments` lists payments received by your node (#2607)
- `closedchannels` lists closed channels. It accepts `--count` and
  `--skip` parameters to limit the number of retrieved items as well
  (#2642)
- `cpfpbumpfees` can be used to unblock chains of unconfirmed
  transactions by creating a child transaction that pays a high fee
  (#1783)
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.

3 participants