Send events when HTLCs settle on-chain #884

pm47 · 2019-03-06T16:43:35Z

This is a simpler alternative to #867, with the following limitations:

no OnChainRefundsDb and associated API calls
PaymentSettlingOnChain/PaymentLostOnChain events will be sent
exactly once per payment and have less information
we don't touch HtlcTimeoutTx
use json4s type hints instead of manual attributes to case classes

This is a simpler alternative to #867, with the following limitations: - no `OnChainRefundsDb` and associated API calls - `PaymentSettlingOnChain` event will be sent exactly once per payment and have less information - we don't touch `HtlcTimeoutTx` - use json4s type hints instead of manual attributes to case classes

# Conflicts: # eclair-core/src/main/scala/fr/acinq/eclair/api/Service.scala # eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentEvents.scala # eclair-core/src/test/scala/fr/acinq/eclair/api/JsonSerializersSpec.scala

araspitzu · 2019-03-28T15:10:50Z

TODO: update the doc

eclair-core/src/main/scala/fr/acinq/eclair/api/Service.scala

pm47

This LGTM to me but is missing a test on PaymentSettlingOnChain. Will take a stab at it

pm47 · 2019-03-28T16:25:30Z

eclair-core/src/main/scala/fr/acinq/eclair/api/Service.scala

-        case received: PaymentReceived => flowInput.offer(received.paymentHash.toString)
+        case message: PaymentFailed => flowInput.offer(Serialization.write(message)(formatsWithTypeHint))
+        case message: PaymentEvent => flowInput.offer(Serialization.write(message)(formatsWithTypeHint))
+        case other => logger.info(s"Unexpected ws message: $other")


Unhandled message should be handled differently

pm47 · 2019-03-29T15:29:30Z

eclair-core/src/main/scala/fr/acinq/eclair/api/JsonSerializers.scala

@@ -186,4 +186,15 @@ object JsonSupport extends Json4sSupport {

  implicit val shouldWritePretty: ShouldWritePretty = ShouldWritePretty.True

+  case class CustomTypeHints(custom: Map[Class[_], String]) extends TypeHints {


Can this be put in a separate file?

I think it belongs here logically, it's where we define our application-wide JsonSupport and all the custom serializers.

pm47 · 2019-03-29T15:29:38Z

eclair-core/src/main/scala/fr/acinq/eclair/api/Service.scala

+  // used to send typed messages over the websocket
+  val formatsWithTypeHint = formats.withTypeHintFieldName("type") +
+    CustomTypeHints(Map(
+      classOf[PaymentSent] -> "payment-sent",


pm47 · 2019-03-29T15:30:57Z

eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala

@@ -1247,6 +1247,11 @@ class Channel(val nodeParams: NodeParams, wallet: EclairWallet, remoteNodeId: Pu
            log.info(s"cannot fail overriden htlc #${add.id} paymentHash=${add.paymentHash} (origin not found)")
        }
      }
+      // for our outgoing payments, let's send events if we know that they will settle on chain
+      Closing
+        .onchainOutgoingHtlcs(d.commitments.localCommit, d.commitments.remoteCommit, d.commitments.remoteNextCommitInfo.left.toOption.map(_.nextRemoteCommit), tx)


@sstone can you double check this? this is quite sensitive

eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala

pm47 · 2019-03-29T15:34:01Z

eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentRequest.scala

@@ -228,10 +228,10 @@ object PaymentRequest {
      f.version match {
        case 17 if prefix == "lnbc" => Base58Check.encode(Base58.Prefix.PubkeyAddress, data)
        case 18 if prefix == "lnbc" => Base58Check.encode(Base58.Prefix.ScriptAddress, data)
-        case 17 if prefix == "lntb" => Base58Check.encode(Base58.Prefix.PubkeyAddressTestnet, data)
-        case 18 if prefix == "lntb" => Base58Check.encode(Base58.Prefix.ScriptAddressTestnet, data)
+        case 17 if prefix == "lntb" || prefix == "lnbcrt" => Base58Check.encode(Base58.Prefix.PubkeyAddressTestnet, data)


@sstone can you double check this?

It's correct because Base58 addresses on testnet and regtest have the same prefix, but not Bech32 addresses :(
But I think handling "lnbcrt" on a separate line looks a bit cleaner

…gy for websocket queue.

…d messages be handled by akka's default

Co-Authored-By: araspitzu <a.raspitzu@protonmail.com>

Co-Authored-By: pm47 <pm47@users.noreply.github.com>

sstone · 2019-03-29T17:21:38Z

eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentRequest.scala

@@ -228,10 +228,10 @@ object PaymentRequest {
      f.version match {
        case 17 if prefix == "lnbc" => Base58Check.encode(Base58.Prefix.PubkeyAddress, data)
        case 18 if prefix == "lnbc" => Base58Check.encode(Base58.Prefix.ScriptAddress, data)
-        case 17 if prefix == "lntb" => Base58Check.encode(Base58.Prefix.PubkeyAddressTestnet, data)
-        case 18 if prefix == "lntb" => Base58Check.encode(Base58.Prefix.ScriptAddressTestnet, data)
+        case 17 if prefix == "lntb" || prefix == "lnbcrt" => Base58Check.encode(Base58.Prefix.PubkeyAddressTestnet, data)


It's correct because Base58 addresses on testnet and regtest have the same prefix, but not Bech32 addresses :(
But I think handling "lnbcrt" on a separate line looks a bit cleaner

sstone · 2019-03-29T17:22:36Z

eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentRequest.scala

        case version if prefix == "lnbc" => Bech32.encodeWitnessAddress("bc", version, data)
-        case version if prefix == "lntb" => Bech32.encodeWitnessAddress("tb", version, data)
+        case version if prefix == "lntb" || prefix == "lnbcrt" => Bech32.encodeWitnessAddress("tb", version, data)


Bech32 prefix should be "bcrt", not "tb"

Done in 5e476fd

…pi-pm

…ress

araspitzu · 2019-04-03T09:35:35Z

LGTM, i'll add the fallback address for /receive in #885

pm47 force-pushed the extended-api-pm branch from 663d111 to ea00496 Compare March 7, 2019 09:51

pm47 force-pushed the extended-api-pm branch from ea00496 to 85df143 Compare March 7, 2019 10:14

pm47 mentioned this pull request Mar 7, 2019

Add a proper payments database #885

Merged

araspitzu added 6 commits March 28, 2019 10:44

Merge branch 'master' into extended-api-pm

8ec8574

# Conflicts: # eclair-core/src/main/scala/fr/acinq/eclair/api/Service.scala # eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentEvents.scala # eclair-core/src/test/scala/fr/acinq/eclair/api/JsonSerializersSpec.scala

Finish merging master

c557f70

Merge branch 'master' into extended-api-pm

cd703f2

Reorganize specialized formats for the websocket

10bc4ab

Remove 'fallbackAddress' from /receive API

5b423b1

remote println from test,

c8d3887

araspitzu changed the title ~~[WIP] Send events when HTLCs settle on-chain~~ Send events when HTLCs settle on-chain Mar 28, 2019

Add test for websocket

9a7e215

pm47 commented Mar 28, 2019

View reviewed changes

eclair-core/src/main/scala/fr/acinq/eclair/api/Service.scala Outdated Show resolved Hide resolved

Use nicer custom type hints when serializing to json (websocket)

52b17fe

pm47 commented Mar 29, 2019

View reviewed changes

araspitzu and others added 5 commits March 29, 2019 16:57

Log error with warn level in websocket actor, use backpressure strate…

09618a8

…gy for websocket queue.

re-introduce dropTail strategy for websocket buffer, let the unhandle…

008b2ca

…d messages be handled by akka's default

Update eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala

14da6d3

Co-Authored-By: araspitzu <a.raspitzu@protonmail.com>

cleaner isSentByLocal

9624383

Co-Authored-By: pm47 <pm47@users.noreply.github.com>

added tests

dd52cdc

sstone requested changes Mar 29, 2019

View reviewed changes

araspitzu added 3 commits March 29, 2019 18:59

Fix bech32 prefix for regtest

5e476fd

Merge remote-tracking branch 'origin/extended-api-pm' into extended-a…

77eb1de

…pi-pm

Separate cases for bech32 testnet and regtest for BOLT11 fallback add…

1b91609

…ress

araspitzu requested a review from sstone April 2, 2019 12:11

Formatting, remove unused param in comments

e3f8d9b

sstone approved these changes Apr 3, 2019

View reviewed changes

araspitzu merged commit 3a56ad9 into master Apr 3, 2019

araspitzu deleted the extended-api-pm branch April 3, 2019 09:41

pm47 mentioned this pull request Apr 15, 2019

Extended API for fine-grained payment tracking #867

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Send events when HTLCs settle on-chain #884

Send events when HTLCs settle on-chain #884

pm47 commented Mar 6, 2019

araspitzu commented Mar 28, 2019

pm47 left a comment

pm47 Mar 28, 2019

pm47 Mar 29, 2019

araspitzu Mar 29, 2019

pm47 Mar 29, 2019

pm47 Mar 29, 2019

sstone Apr 3, 2019

pm47 Mar 29, 2019

sstone Mar 29, 2019

sstone Mar 29, 2019

sstone Mar 29, 2019

araspitzu Mar 29, 2019

araspitzu commented Apr 3, 2019

		@@ -186,4 +186,15 @@ object JsonSupport extends Json4sSupport {

		implicit val shouldWritePretty: ShouldWritePretty = ShouldWritePretty.True

		case class CustomTypeHints(custom: Map[Class[_], String]) extends TypeHints {

Send events when HTLCs settle on-chain #884

Send events when HTLCs settle on-chain #884

Conversation

pm47 commented Mar 6, 2019

araspitzu commented Mar 28, 2019

pm47 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

araspitzu commented Apr 3, 2019