0.9.0 #23
Merged
0.9.0 #23
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The external Makefile hardcodes gcc to get the machine spec. This should use the configured C compiler instead. This bug was introduced in 6014644.
Quote marks are not special to make: as it can't find external/"x86_64-linux-gnu"/libwally-core-build/src/libwallycore.la it always insists on rebuilding it (which rebuilds the world). If we have spaces in TARGET_DIR, we're in trouble already. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-None
Unused here, but we'll use it in the next commit so that we can always pass back the effective / used feerate to the caller of `reserveinputs` This makes opening a channel much easier if we've internally determined the feerate
Reserve and unreserve wallet UTXOs using a PSBT which includes those inputs. Note that currently we unreserve inputs everytime the node restarts. This will be addressed in a future commit. Changelog-Added: JSON-RPC: Adds two new rpc methods, `reserveinputs` and `unreserveinputs`, which allow for reserving or unreserving wallet UTXOs
the bitcoin_tx version is basically a wrapper for the wally_tx script extraction -- here we pull it apart so we can easily get a tal'd script for a wally_tx_output
With the incursion of PSBTs, we're moving away from bitcoin_tx
Changelog-Added: JSON-RPC: new call `signpsbt` which will add the wallet's signatures to a provided psbt Changelog-Added: JSON-RPC: new call `sendpsbt` which will finalize and send a signed PSBT
when attempting to calculate the fees for a tx where we don't own all of the outputs, we can overshoot the feerate
Our existing coin_moves tracking logic assumed that any tx we had an input in belonged to *all* of our wallet (not a bad assumption as long as there was no way to update a tx that spends our wallets) Now that we've got `signpsbt` implemented, however, we need to be careful about how we account for withdrawals. For now we do a best guess at what the feerate is, and lump all of our spent outputs as a 'withdrawal' when it's impossible to disambiguate
rusty$ make prod-release make: *** No rule to make target 'test', needed by 'prod-release'. Stop. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Detection of reserved outputs is async, going via bitcoind. Wait for them. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If the daemon already knows about the channel before it was stopped, it won't get this message from gossipd. That's OK, since we explicitly test for the channel being active two lines down. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-None
"backtrace": [
"ccan/ccan/tal/tal.c:442 (tal_alloc_)",
"wallet/wallet.c:154 (wallet_stmt2output)",
"wallet/wallet.c:275 (wallet_get_utxos)",
"wallet/wallet.c:3792 (wallet_clean_utxos)",
"lightningd/lightningd.c:914 (main)"
],
"label": "wallet/wallet.c:154:struct utxo",
"parents": [
"wallet/wallet.c:273:struct utxo*[]"
],
"value": "0x24c1be8"
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We were assuming `wallet_channel_insert` that there cannot be a matching peer if our in-memory representation isn't bound to it (`dbid == 0`). If we then attempt to create the peer, and we already had one it'd cause a unique constraint violation. As far as I can tell this could end up happening if we have an uncommitted channel, and then exited without cleanup (`tal_destructor` on the uncommitted channel not running). This could then leave the peer in the DB. This is because the constraint that every peer has at least one channel is not enforce at DB level, but rather in destructors that may or may not run. Changelog-Fixed: Fixed a failing assertion if we reconnect to a peer that we had a channel with before, and then attempt to insert the peer into the DB twice.
Changelog-Added: We now install `lightning-hsmtool` for your `hsm_secret` needs. See: ElementsProject#3717 (comment) It seems reasonable to add this to the standard install, and to document it properly as well, hopefully we can fill in the documentation better later on.
…h V3 onions Changelog-None Signed-off-by: Saibato <saibato.naga@pm.me>
…ct fail. @thestick613 noticed that since tor version below 0.3.2.2-alpha will not support V3 ed25519 address formats, the error handling is not that helpful in the error message from cli. So now we add an hint. Changelog-None: Signed-off-by: Saibato <saibato.naga@pm.me> connectd/connectd.h; Add helper function to update conn error list Signed-off-by: Saibato <saibato.naga@pm.me>
Changelog-None: Signed-off-by: Saibato <saibato.naga@pm.me>
The actual steps are mocked out, but we can already run through the various phases of a payment, and the modifiers should be called for each state.
This commit can be reverted/skipped once we have implemented all the logic and have feature parity with the normal `pay`. It's main purpose is to expose the unfinished functionality to test it, without completely breaking the existing `pay` command.
This should make it easy for JSON-RPC functions and modifiers to get the associated data for a given modifier name. Useful if a modifier needs to access its parent's modifier data, or in other functions that need to access modifier data, e.g., when passing destination pointers into the `param()` call.
This is likely a bit of overkill for this type of functionality, but it is a nice first use-case of how functionality can be compartmentalized into modifiers. If makes swapping retry mechanisms in and out really simple.
A payment is considered finished if it is in a final state (success or failure) or all its sub-payments are finished. If that's the case we notify `payment_finished` and bubble up through `payment_child_finished`, eventually bubbling up to the root, which can then report success of failure back to the RPC command that initiated the whole process.
This is necessary so we can build the absolute locktimes in the next step. For now just fetch the blockheight on each (sub-)payment, later we can reuse the root blockheight if it ends up using too much traffic.
Te `sendonion` docs where claiming that the `first_hop` needs to specify a `channel_id` whereas it should really be the `node_id` of the peer we are trying to contact.
We're using it in a couple of places to see if we even performed the attempt, so we need to make sure it's initialized.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This does two things: it checks if the destination of the payment is at all reachable without routehints, and if it is it adds a direct attempt as option to the routehints in the form of a NULL routehint. It also simplifies the selection of the routehint since the direct case is no longer special, instead we just return a NULL routehint as if it were a normal routehint.
It was spread over the step callback, but we only need to initialize the routehints array there, child-payments can just inherit most of the information.
We have two places we need to do that now: in the root payment after we checked if the destination is reachable, and in any other payment directly in the initialization-step callback.
We store an offset of the current routehint in the modifier data. It gets incremented on retry, and it gets reset to 0 on split. This is because once we split we have a different amount and a previously unusable routehint becomes usable again.
This uses @cdecker's idea of excluding the routehinted channel from the route, and also consumes the route hints as it goes so that it makes progress. I don't know if this is correct, but it reliably passes tests/test_pay.py::test_tlv_or_legacy now.
The child payments will sometimes depend on the step of the parent, and making sure that the parent state is correct before we create the children is therefore important.
The shortcut in the retry_mod that we can skip retrying if getroute fails or we have no result is only valid if the parameters don't change. As we iterate through the routehints the parameters change, and so we must signal to the retry_mod that it can retry even in those cases.
jsonrpc_stream_fail already adds a message field.
[Separated into separate commit by RR]
And document the partid arg. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Changelog-Added: JSON-RPC: `sendonion` has a new optional `bolt11` argument for when it's used to pay an invoice.
…ay`) [ Extracted into standalone patch and comment added by RR ]
There were no channel updates in my log; because sendonion doesn't know the actual node_ids or channel_ids, we can't tell gossipd what node/channel it was so it can no longer remove them on PERM errors. However, we can tell it the error message so it can apply the update. Fixes: ElementsProject#3877 Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-None
Since we started using `sendonion` in the `pay` plugin we no longer automatically have the `amount` annotation on (partial) payments. This replicates the issue so we can fix it. Reported-by: Rusty Russell <@rustyrussell>
These were spamming my logs and could result in misleading results being returned on `listpays` and `listsendpays`.
While not directly necessary, it still feeds the `listpays` result, and so we should pass it along if we can, so we don't have to rely solely on the `amount_sent` field, which includes the fees. Reported-by: Rusty Russell <@rustyrussell>
We sum up the amounts just like we do with `amount_sent`, however we may have parts that don't have that annotation, so make it optional. Suggested-by: Rusty Russell <@rustyrussell>
```
# Excludes channel, then ignores routehint which includes that, then
# it excludes other channel.
> assert len(status) == 2
E assert 1 == 2
E -1
E +2
```
The invoice we use at the end has a routehint: 50% of the time it's
to l2 (which fails), 50% to l5 (which succeeds).
Change it to create invoice before channel with l5 so it does the
retry like we expect here.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
we're about to add a migration that requires access to the bip32_key in order to calculate missing scriptpubkeys. prior to this patch, we don't have access to the bip32 key in the db migration, as it's set on the wallet but after the db migrations are run. here we patch it through so that every migration can access it
the way we use PSBTs to sign things requires that we have the scriptpubkey available on the utxo so we can populate the witness-utxo field with it. this causes problems if we don't already have the scriptpubkey cached in the database, as in *some* cases we require a round trip to the HSM to populate them to get over this hump, we backfill any and all missing scriptpubkey information for the utxo's that we hold in our wallet. this will allow us to clean up the NULL handling of missing scriptpubkeys.
Now that we're *guaranteed* to have a scriptpubkey entry in the database, we remove the NULL handling for it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.