Skip to content

Store failure messages for outgoing payments#1074

Closed
rorp wants to merge 6 commits intoACINQ:masterfrom
rorp:outgoing_payment_failures
Closed

Store failure messages for outgoing payments#1074
rorp wants to merge 6 commits intoACINQ:masterfrom
rorp:outgoing_payment_failures

Conversation

@rorp
Copy link
Contributor

@rorp rorp commented Jul 14, 2019

Currently there's no way to get payment failures using getsentinfo RPC. This PR addresses that.

@codecov-io
Copy link

codecov-io commented Jul 14, 2019

Codecov Report

Merging #1074 into master will decrease coverage by 0.08%.
The diff coverage is 84.84%.

@@            Coverage Diff             @@
##           master    #1074      +/-   ##
==========================================
- Coverage    82.7%   82.62%   -0.09%     
==========================================
  Files         101      101              
  Lines        7621     7659      +38     
  Branches      293      302       +9     
==========================================
+ Hits         6303     6328      +25     
- Misses       1318     1331      +13
Impacted Files Coverage Δ
...src/main/scala/fr/acinq/eclair/db/PaymentsDb.scala 100% <ø> (ø) ⬆️
...c/main/scala/fr/acinq/eclair/payment/Relayer.scala 88.11% <0%> (ø) ⬆️
.../src/main/scala/fr/acinq/eclair/db/Databases.scala 100% <100%> (ø) ⬆️
...a/fr/acinq/eclair/db/sqlite/SqlitePaymentsDb.scala 99.29% <100%> (+0.17%) ⬆️
...ala/fr/acinq/eclair/payment/PaymentLifecycle.scala 88.42% <83.33%> (-0.07%) ⬇️
...lockchain/bitcoind/rpc/ExtendedBitcoinClient.scala 84.61% <0%> (-7.7%) ⬇️
.../acinq/eclair/blockchain/bitcoind/ZmqWatcher.scala 92.3% <0%> (-3.85%) ⬇️
...clair/blockchain/electrum/ElectrumClientPool.scala 75.26% <0%> (-3.23%) ⬇️
...c/main/scala/fr/acinq/eclair/channel/Channel.scala 83.82% <0%> (-0.74%) ⬇️
...src/main/scala/fr/acinq/eclair/router/Router.scala 85.81% <0%> (-0.48%) ⬇️
... and 3 more

Copy link
Contributor

@araspitzu araspitzu left a comment

Choose a reason for hiding this comment

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

The concept is ACK, however storing remotely generated strings in the database represents a concern. Even though it's technically safe thanks to preparedStatement we need to be extra careful when altering the payment db since it contains our preimages.

@rorp
Copy link
Contributor Author

rorp commented Jul 20, 2019

We can create a separate DB for remotely generated strings and/or use something like org.apache.commons.text.StringEscapeUtils to sanitize them.

@t-bast
Copy link
Member

t-bast commented Sep 20, 2019

This change has been included in #1130 🎉

@t-bast t-bast closed this Sep 20, 2019
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.

4 participants