deleteinvoice method

[rorp]

This is needed for automated rebalancing too.

[codecov-commenter]

Codecov Report

Merging #1984 (6082a6b) into master (df63ea4) will decrease coverage by 0.07%.
The diff coverage is 86.20%.

@@            Coverage Diff             @@
##           master    #1984      +/-   ##
==========================================
- Coverage   87.68%   87.61%   -0.08%     
==========================================
  Files         160      160              
  Lines       12533    12559      +26     
  Branches      530      550      +20     
==========================================
+ Hits        10990    11003      +13     
- Misses       1543     1556      +13     

Impacted Files	Coverage Δ
...r-core/src/main/scala/fr/acinq/eclair/Eclair.scala	48.40% <0.00%> (-0.63%)	⬇️
.../main/scala/fr/acinq/eclair/db/DualDatabases.scala	0.00% <0.00%> (ø)
...src/main/scala/fr/acinq/eclair/db/PaymentsDb.scala	92.85% <ø> (ø)
...ain/scala/fr/acinq/eclair/db/pg/PgPaymentsDb.scala	86.75% <100.00%> (+0.76%)	⬆️
...a/fr/acinq/eclair/db/sqlite/SqlitePaymentsDb.scala	98.21% <100.00%> (+0.08%)	⬆️
...clair/blockchain/bitcoind/rpc/BatchingClient.scala	86.36% <0.00%> (-13.64%)	⬇️
...ala/fr/acinq/eclair/balance/ChannelsListener.scala	93.10% <0.00%> (-3.45%)	⬇️
.../acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala	85.59% <0.00%> (-2.55%)	⬇️
...cala/fr/acinq/eclair/payment/relay/NodeRelay.scala	95.93% <0.00%> (-1.63%)	⬇️
...cinq/eclair/payment/receive/MultiPartHandler.scala	95.20% <0.00%> (-1.60%)	⬇️
... and 4 more

[t-bast]

Thanks for doing this!
Can you add this to the release notes?

[t-bast]

Thanks, we're getting closer!
My last pending issue is that this introduces a race condition in MultiPartHandler.scala.

We previously never removed an invoice from the DB, so we assumed we could do db,getIncomingPayment and if it returned something, we could process the payment and then call db.receiveIncomingPayment and it would always succeed.

Unfortunately the call to db.receiveIncomingPayment may now fail if the invoice is deleted concurrently. We should change the signature from:

/**
   * Mark an incoming payment as received (paid). The received amount may exceed the payment request amount.
   * Note that this function assumes that there is a matching payment request in the DB.
   */
  def receiveIncomingPayment(paymentHash: ByteVector32, amount: MilliSatoshi, receivedAt: Long = System.currentTimeMillis): Unit

to:

/**
   * Mark an incoming payment as received (paid). The received amount may exceed the payment request amount.
   * If there was no matching payment request in the DB, this will return false.
   */
  def receiveIncomingPayment(paymentHash: ByteVector32, amount: MilliSatoshi, receivedAt: Long = System.currentTimeMillis): Boolean

Then in MultiPartHandler.scala, we should check the result of db.receiveIncomingPayment and only release the preimage if it was true.

Please also add tests for this last change.

[t-bast]

Thanks!

I applied some refactoring and fixed the failing test on top of your work in 8d49fc1, can you cherry-pick this commit? After that we should be good to go.

One thing I wanted to highlight that may not be obvious: using a Try[T] is very different from throwing. It is purely functional and doesn't capture an expensive stack trace, which is why it should always be preferred to actually throwing (and why I'm such a pita when it comes to throwing exceptions).