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 a proper payments database #885

Merged
merged 93 commits into from Apr 16, 2019

Conversation

Projects
None yet
6 participants
@pm47
Copy link
Member

commented Mar 7, 2019

This builds on #884.

There is no unique identifier for payments in LN protocol. Critically,
we can't use payment_hash as a unique id because there is no way to
ensure unicity at the protocol level.

Also, the general case for a "payment" is to be associated to multiple
update_add_htlcs, because of automated retries. We also routinely
retry payments, which means that the same payment_hash will be
conceptually linked to a list of lists of update_add_htlcs.

In order to address this, we introduce a payment id, which uniquely
identifies a payment, as in a set of sequential update_add_htlc
managed by a single PaymentLifecycle that ends with a PaymentSent or
PaymentFailed outcome.

We can then query the api using either payment_id or payment_hash.
The former will return a single payment status, the latter will return a
set of payment statuses, each identified by their payment_id.

add a payment identifier
This is in preparation for a proper payments database.

There is no unique identifier for payments in LN protocol. Critically,
we can't use `payment_hash` as a unique id because there is no way to
ensure unicity at the protocol level.

Also, the generatl case for a "payment" is to be associated to multiple
`update_add_htlc`s, because of automated retries. We also routinely
retry payments, which means that the same `payment_hash` will be
conceptually linked to a list of lists of `update_add_htlc`s.

In order to address this, we introduce a payment id, which uniquely
identifies a payment, as-in a set of sequential `update_add_htlc`
managed by a single `PaymentLifecycle` that ends with a `PaymentSent` or
`PaymentFailed` outcome.

We can then query the api using either `payment_id` or `payment_hash`.
The former will return a single payment status, the latter will return a
set of payment statuses, each identified by their `payment_id`.

NB: This is just the first step, there is no payment db yet and the ids
are not actually used. Some db tests do not pass.

@pm47 pm47 changed the title Add a proper payments database [WIP] Add a proper payments database Mar 7, 2019

@pm47

This comment has been minimized.

Copy link
Member Author

commented Mar 7, 2019

NB: this is best reviewed using a diff against #884.

pm47 and others added some commits Mar 7, 2019

add a payment identifier
This is in preparation for a proper payments database.

There is no unique identifier for payments in LN protocol. Critically,
we can't use `payment_hash` as a unique id because there is no way to
ensure unicity at the protocol level.

Also, the generatl case for a "payment" is to be associated to multiple
`update_add_htlc`s, because of automated retries. We also routinely
retry payments, which means that the same `payment_hash` will be
conceptually linked to a list of lists of `update_add_htlc`s.

In order to address this, we introduce a payment id, which uniquely
identifies a payment, as-in a set of sequential `update_add_htlc`
managed by a single `PaymentLifecycle` that ends with a `PaymentSent` or
`PaymentFailed` outcome.

We can then query the api using either `payment_id` or `payment_hash`.
The former will return a single payment status, the latter will return a
set of payment statuses, each identified by their `payment_id`.

NB: This is just the first step, there is no payment db yet and the ids
are not actually used. Some db tests do not pass.

# Conflicts:
#	eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelTypes.scala
#	eclair-core/src/main/scala/fr/acinq/eclair/db/sqlite/SqliteAuditDb.scala
#	eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentEvents.scala
#	eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentLifecycle.scala
#	eclair-core/src/main/scala/fr/acinq/eclair/payment/Relayer.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/api/JsonSerializersSpec.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/channel/states/StateTestsHelperMethods.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/db/ChannelStateSpec.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/db/SqliteAuditDbSpec.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentLifecycleSpec.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/payment/RelayerSpec.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/wire/ChannelCodecsSpec.scala
Merge branch 'payments-db-extended' into payments-db
# Conflicts:
#	eclair-core/src/main/scala/fr/acinq/eclair/channel/Channel.scala
#	eclair-core/src/main/scala/fr/acinq/eclair/channel/ChannelTypes.scala
#	eclair-core/src/main/scala/fr/acinq/eclair/db/sqlite/SqliteAuditDb.scala
#	eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentEvents.scala
#	eclair-core/src/main/scala/fr/acinq/eclair/payment/PaymentLifecycle.scala
#	eclair-core/src/main/scala/fr/acinq/eclair/payment/Relayer.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/api/JsonSerializersSpec.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/channel/states/e/NormalStateSpec.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/channel/states/f/ShutdownStateSpec.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/channel/states/g/NegotiatingStateSpec.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/channel/states/h/ClosingStateSpec.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/db/ChannelStateSpec.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/db/SqliteAuditDbSpec.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/payment/ChannelSelectionSpec.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/payment/PaymentLifecycleSpec.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/payment/RelayerSpec.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/wire/ChannelCodecsSpec.scala
@araspitzu

This comment has been minimized.

Copy link
Member

commented Apr 2, 2019

The /send API now immediately returns a UUID, outgoing payments are stored in the paymentsDB and associated with a status, the status is updated every time there is a change by the payment lifecycle. There is also a /paymentinfo API that can be queried via payment-id and returns its status along with some extra info.

araspitzu added some commits Apr 3, 2019

Merge branch 'master' into payments-db
# Conflicts:
#	eclair-core/src/main/scala/fr/acinq/eclair/Eclair.scala
#	eclair-core/src/main/scala/fr/acinq/eclair/channel/Helpers.scala
@araspitzu

This comment has been minimized.

Copy link
Member

commented Apr 3, 2019

The PR also adds an optional fallback address to the /receive API, see b43ec23

@araspitzu

This comment has been minimized.

Copy link
Member

commented Apr 3, 2019

TODO: update the doc

@araspitzu araspitzu changed the title [WIP] Add a proper payments database Add a proper payments database Apr 3, 2019

pm47 and others added some commits Apr 12, 2019

@pm47

This comment has been minimized.

Copy link
Member Author

commented Apr 12, 2019

Serialization result is weird. Why the nulls?

{
    "prefix": "lntb",
    "amount": null,
    "timestamp": 1555089352,
    "nodeId": "03dca9db59c25bfb430667d5c080b55a08b808b39251ec9e5b3d86939262d18e6a",
    "description": "test",
    "paymentHash": "b80ea80ec5d6d1182ea0fa418c7c02ddac3e7d6af686449a46450712155ba2a5",
    "expiry": 3600,
    "minFinalCltvExpiry": null,
    "serialized": "lntb1pwtp37gpp5hq82srk96mg3st4qlfqcclqzmkkrult276ryfxjxg5r3y92m52jsdq8w3jhxaqxqrrssf8gdlty0v67dq3xn6m4e6je84ym65p5uf3a5pmqrlmdyk0ch93ak2pndqhqf5khswq32wmhwq0xyc22qpx4f0zpk77nvq55s37x5sdqqqfwpkw"
}
@n1bor

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

my issues all fixed.

One other point though sentinfo does not return the pre-image? Looks like it is missing from the OutgoingPayment class?
The pre-image is obviously really useful to prove you made the payment. Is in the GUI just not the sent_payments table.

araspitzu added some commits Apr 15, 2019

araspitzu added some commits Apr 15, 2019

Merge branch 'master' into payments-db
# Conflicts:
#	eclair-core/src/main/scala/fr/acinq/eclair/Eclair.scala
#	eclair-core/src/main/scala/fr/acinq/eclair/api/Service.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/api/ApiServiceSpec.scala
#	eclair-core/src/test/scala/fr/acinq/eclair/integration/IntegrationSpec.scala
@pm47
Copy link
Member Author

left a comment

This LGTM provided that we increase the test coverage with #938.

@pm47 pm47 requested a review from sstone Apr 15, 2019

@pm47 pm47 assigned sstone and unassigned pm47 Apr 15, 2019

@Kukks

This comment has been minimized.

Copy link
Contributor

commented Apr 15, 2019

Looking forward to this :D

statement.executeUpdate("CREATE TABLE IF NOT EXISTS payments (payment_hash BLOB NOT NULL PRIMARY KEY, amount_msat INTEGER NOT NULL, timestamp INTEGER NOT NULL)")
require(getVersion(statement, DB_NAME, CURRENT_VERSION) <= CURRENT_VERSION) // version 2 is "backward compatible" in the sense that it uses separate tables from version 1. There is no migration though
statement.executeUpdate("CREATE TABLE IF NOT EXISTS received_payments (payment_hash BLOB NOT NULL PRIMARY KEY, preimage BLOB NOT NULL, payment_request TEXT NOT NULL, received_msat INTEGER, created_at INTEGER NOT NULL, expire_at INTEGER, received_at INTEGER)")
statement.executeUpdate("CREATE TABLE IF NOT EXISTS sent_payments (id TEXT NOT NULL PRIMARY KEY, payment_hash BLOB NOT NULL, preimage BLOB, amount_msat INTEGER NOT NULL, created_at INTEGER NOT NULL, completed_at INTEGER, status VARCHAR NOT NULL)")

This comment has been minimized.

Copy link
@sstone

sstone Apr 16, 2019

Member

There should be an index on payment_hash too

This comment has been minimized.

Copy link
@araspitzu

araspitzu Apr 16, 2019

Member

Agreed, done in f7fc22c


override def listPaymentRequests(from: Long, to: Long): Seq[PaymentRequest] = listPaymentRequests(from, to, pendingOnly = false)

override def listPendingPaymentRequests(from: Long, to: Long): Seq[PaymentRequest] = listPaymentRequests(from, to, pendingOnly = true)

This comment has been minimized.

Copy link
@sstone

sstone Apr 16, 2019

Member

It would make sense to sort returned results by timestamp

This comment has been minimized.

Copy link
@araspitzu

araspitzu Apr 16, 2019

Member

Good one, done in 153b62c

araspitzu added some commits Apr 16, 2019

@sstone

sstone approved these changes Apr 16, 2019

@pm47 pm47 merged commit 9032da5 into master Apr 16, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@pm47 pm47 deleted the payments-db branch Apr 16, 2019

@Kukks Kukks referenced this pull request Apr 18, 2019

Closed

API Callback/Notifications #831

pm47 added a commit that referenced this pull request May 21, 2019

Merge master into Android (#1002)
* Fix eclair-cli to work with equal sign in arguments (#926)

* Fix eclair cli argument passing

* Modify eclair-cli to work with equals in arguments

* Eclair-cli: show usage when wrong params are received

* Remove deprecated call from eclair-cli help message [ci skip]

* Make Electrum tests pass on windows (#932)

There was an obscure Docker error when trying to start an Electrum
server in tests. [1]

It appears that there is a conflict between Docker and Hyper-V on some
range of ports.

A workaround is to just change the port we were using.

[1] docker/for-win#3171

* API: fix fee rate conversion (#936)

Our `open` API calls expects an optional fee rate in satoshi/byte, which is the most widely
used unit, but failed to convert to satoshi/kiloweight which is the standard in LN.
We also check that the converted fee rate cannot go below 253 satoshi/kiloweight.

* Expose the websocket over HTTP GET to work properly with basic auth (#934)

* Expose the websocket over HTTP GET
* Add test for basic auth over websocket endpoint

* Set max payment attempts from configuration (#931)

With a default to `5`.

* Add a proper payments database (#885)

There is no unique identifier for payments in LN protocol. Critically,
we can't use `payment_hash` as a unique id because there is no way to
ensure unicity at the protocol level.

Also, the general case for a "payment" is to be associated to multiple
`update_add_htlc`s, because of automated retries. We also routinely
retry payments, which means that the same `payment_hash` will be
conceptually linked to a list of lists of `update_add_htlc`s.

In order to address this, we introduce a payment id, which uniquely
identifies a payment, as in a set of sequential `update_add_htlc`
managed by a single `PaymentLifecycle` that ends with a `PaymentSent` or
`PaymentFailed` outcome.

We can then query the api using either `payment_id` or `payment_hash`.
The former will return a single payment status, the latter will return a
set of payment statuses, each identified by their `payment_id`.

* Add a payment identifier

* Remove InvalidPaymentHash channel exception

* Remove unused 'close' from paymentsDb

* Introduce sent_payments in PaymentDB, bump db version

* Return the UUID of the ongoing payment in /send API

* Add api to query payments by ID

* Add 'fallbackAddress' in /receive API

* Expose /paymentinfo by paymentHash

* Add id column to audit.sent table, add test for db migration

* Add invoices to payment DB

* Add license header to ExtraDirective.scala

* Respond with HTTP 404 if the corresponding invoice/paymentHash was not found.

* Left-pad numeric bolt11 tagged fields to have a number of bits multiple of five (bech32 encoding).

* Add invoices API

* Remove CheckPayment message

* GUI: consume UUID reply from payment initiator

* API: reply with JSON encoded response if the queried element wasn't found

* Return a payment request object in /receive

* Remove limit of pending payment requests!

* Avoid printing "null" fields when serializing an invoice to json

* Add index on paymentDb.sent_payments.payment_hash

* Order results in descending order in listPaymentRequest

* Electrum: do not persist transaction locks (#953)

Locks held on utxos that are used in unpublished funding transactions should not be persisted.
If the app is stopped before the funding transaction has been published the channel is forgotten
and so should be locks on its funding tx utxos.

* Added a timeout for channel open request (#928)

Until now, if the peer is unresponsive (typically doesn't respond to
`open_channel` or `funding_created`), we waited indefinitely, or until the
connection closed.

It translated to an API timeout for users, and uncertainty about the
state of the channel.

This PR:
- adds an optional `--openTimeoutSeconds` timeout to the `open` endpoint, that will
actively cancel the channel opening if it takes too long before reaching
state `WAIT_FOR_FUNDING_CONFIRMED`.
- makes the `ask` timeout configurable per request with a new `--timeoutSeconds`
- makes the akka http timeout slightly greater than the `ask` timeout

Ask timeout is set to 30s by default.

* Set `MAX_BUFFERED` to 1,000,000 (#948)

Note that this doesn't mean that we will buffer 1M objects in memory:
those are just pointers to (mostly) network announcements that already
exist in our routing table.

Routing table has recently gone over 100K elements (nodes,
announcements, updates) and this causes the connection to be closed when
peer requests a full initial sync.

* Fix Dockerfile maven binary checksum (#956)

The Maven 3.6.0 SHA256 checksum was invalid and caused the docker build to fail.

* Add channel errors in audit db (#955)

We now keep track of all local/remote channel errors in the audit db.

* Added simple plugin support (#927)

Using org.clapper:classutil library and a very simple `Plugin` interface.

* Live channel database backup (#951)

* Backup running channel database when needed

Every time our channel database needs to be persisted, we create a backup which is always
safe to copy even when the system is busy.

* Upgrade sqlite-jdbc to 3.27.2.1

* BackupHandler: use a specific bounded mailbox

BackupHandler is now private, users have to call BackupHandler.props() which always
specifies our custom bounded maibox.

* BackupHandler: use a specific threadpool with a single thread

* Add backup notification script

Once a new backup has been created, call an optional user defined script.

* Update readme with bitcoin 0.17 instructions (#958)

This has somehow been missed by PR #826.

* Backup: explicitely specify move options (#960)

* Backup: explicitely specify move options

We now specify that we want to atomically overwrite the existing backup file with the new one (fixes
a potential issue on Windows).
We also publish a specific notification when the backup process has been completed.

* Print stack trace when crashing during boot sequence (#949)

* Print stack trace when crashing during boot sequence

* Use friendly message when db compatibility check fails

* ElectrumWallet should not send ready if syncing (#963)

This commit is already embedded in version `0.2-android-beta22`.

* Channel: Log additional data (#943)

* Channel: Log additional data

Log local channel parameters, and our peer's open or accept message.
This should be enough to recompute keys needed to recover funds in case of unilateral close.

* Electrum: make debug logs shorter (#964)

* Better handling of closed channels (#944)

* Remove closed channels when application starts

If the app is stopped just after a channel has transition from CLOSING to CLOSED, when the  application starts again if will be restored as CLOSING. This commit checks channel data and remove closed channels instead of restoring them.

* Channels Database: tag closed channels but don't delete them

Instead we add a new `closed` column that we check when we restore channels.

* Document how we check and remove closed channels on startup

* Do not print the stacktrace on stderr when there is an error at boot (#966)

* Do not print the stacktrace on stdout when there is an error at boot

* Fix flaky test in PaymentLifecycleSpec (#967)

* Use local random pamentHash for each test in paymentlifecyclespec, intercept the route request before the router.

* Rename `eclair.bak` to `eclair.sqlite.bak` (#968)

This removes any ambiguity about what the content of the file is about.

* Fixed concurrency issue in `IndexedObservableList` (#961)

Update map with new indexes after element is removed

Fixes #915

* Various fix and improvements in time/timestamp handling (#971)

This PR standardizes the way we compute the current time as unix timestamp 

- Scala's Platform is used and the conversion is done via scala's concurrent.duration facilities
- Java's Instant has been replaced due to broken compatibility with android
- AuditDB events use milliseconds (fixes #970)
- PaymentDB events use milliseconds
- Query filters for AuditDB and PaymentDB use seconds

* API: Support query by `channelId` or `shortChannelId` everywhere (#969)

Add support for querying a channel information by its `shortChannelId`.

* Smarter strategy for sending `channel_update`s (#950)

The goal is to prevent sending a lot of updates for flappy channels.

Instead of sending a disabled `channel_update` after each disconnection, 
we now wait for a payment to try to route through the channel and only 
then reply with a disabled `channel_update` and broadcast it on the
network.

The reason is that in case of a disconnection, if noone cares about that
channel then there is no reason to tell everyone about its current
(disconnected) state.

In addition to that, when switching from `SYNCING`->`NORMAL`, instead
of emitting a new `channel_update` with flag=enabled right away, we wait
a little bit and send it later. We also don't send a new `channel_update` if
it is identical to the previous one (except if the previous one is outdated).

This way, if a connection to a peer is unstable and we keep getting
disconnected/reconnected, we won't spam the network.

The extra delay allows us to remove the change made in #888, which was
a workaround in case we generated `channel_update` too quickly.

Also, increased refresh interval from 7 days to 10 days. There was no
need to be so conservative.

Note that on startup we still need to re-send `channel_update` for all 
channels in order to properly initialize the `Router` and the `Relayer`.
Otherwise they won't know about those channels, and e.g. the 
`Relayer` will return `UnknownNextPeer` errors.

But we don't need to create new `channel_update`s in most cases, so 
this should have little or no impact to gossip because our peers will
already know the updates and will filter them out.

On the other hand, if some global parameters (like relaying fees) are
changed, it will cause the creation a new `channel_update` for all
channels.

* Fixed overflow issue with max duration (#975)

This is a regression caused by #971, because `Duration` has a max value of `Long.MaxValue` *nanoseconds*, not *seconds*.

* Use proper closing type in `ChannelClosed` event (#977)

There was actually a change introduced by #944 where we used
`ClosingType.toString` instead of manually defining types, causing a
regression in the audit database.

* Update bash autocompletion for eclair-cli (#983)

* Update bash autocompletition file to suggest all the endpoints

* Update list of commands in eclair-cli help message

* Replace `UnknownPaymentHash` and `IncorrectPaymentAmount` with `IncorrectOrUnknownPaymentDetails` (#984)

See lightningnetwork/lightning-rfc#516 and lightningnetwork/lightning-rfc#544

* Wireshark dissector support (#981)

* Transport: add support for encryption key logging.
This is the format the wireshark lightning-dissector uses to be able to decrypt lightning messages.

* Enrich test for internal eclair API implementation (fr.acinq.eclair.Eclair.scala) (#938)

* Add test to EclairImpl for `/send`, `/allupdates` and `/forceclose/`

* Set default chain to "mainnet" (#989)

Eclair is now configured to run on mainnet by default.

* Set tcp client timeout to 20s (#990)

So that it fails before the ask/api time out.

* Add bot support for code coverage (codecov) (#982)

* Add scoverage-maven-plugin dependency

* Update travis build to generate a scoverage report

* Add custom codecov configuration to have nice PR comments

* Add badge for test coverage in readme

* Accept `commit_sig` without changes (#988)

LND sometimes sends a new signature without any changes, which is a
(harmless) spec violation.

Note that the test was previously not failing because it wasn't specific
enough. The test now fails and has been ignored.

* Ignore subprojects eclair-node/eclair-node-gui in the codecov report (#991)

* Use bitcoind fee estimator first (#987)

* use bitcoind fee provider first

* set default `smooth-feerate-window`=6

* Configuration: increase fee rate mismatch threshold

We wil accept fee rates that up to 8x bigger or smaller than our local fee rate

* Updated license header (#992)

* Release v0.3 (#994)

* gui: include javafx native libraries for windows, mac, linux

* Release v0.3

* Set version to 0.3.1-SNAPSHOT

* Improved test coverage of `io` package (#996)

* improved test coverage of `NodeURI`

* improved test coverage of `Peer`

* Fix TextUI

* BackupHandler: use renameTo() on Android

Most Path methods are not available at our current API level
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.