0.8.2 #21
Merged
0.8.2 #21
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
Don't let make pollute subprojects' environment with our own `CFLAGS`,
which are quite strict because that breaks at least libwally-core:
```sh
$ ./configure ...
$ CFLAGS=whatever_this_is_irrelevant make
...
cd external/libwally-core-build && ../libwally-core/configure ...
...
CFLAGS = -DBINTOPKGLIBEXECDIR="\"../libexec/c-lightning\"" -Wall -Wundef -Wmissing-prototypes -Wmissing-declarations -Wstrict-prototypes -Wold-style-definition -Werror -std=gnu11 -g -fstack-protector -I ccan -I external/libwally-core/include/ -I external/libwally-core/src/secp256k1/include/ -I external/jsmn/ -I external/libbacktrace/ -I external/libbacktrace-build -I . -I/usr/local/include -DCCAN_TAKE_DEBUG=1 -DCCAN_TAL_DEBUG=1 -DCCAN_JSON_OUT_DEBUG=1 -DSHACHAIN_BITS=48 -DJSMN_PARENT_LINKS -DBUILD_ELEMENTS=1 -W -std=c89 -pedantic -Wall -Wextra -Wcast-align -Wnested-externs -Wshadow -Wstrict-prototypes -Wno-unused-function -Wno-long-long -Wno-overlength-strings -fvisibility=hidden -O3
...
In file included from ../../libwally-core/src/base58.c:4:
../../libwally-core/src/ccan/ccan/endian/endian.h:71:24: error: unused function 'bswap_16'
[-Werror,-Wunused-function]
static inline uint16_t bswap_16(uint16_t val)
^
```
If `CFLAGS` is set in its environment, then `make` would export our own
`CFLAGS` to any subprocesses it starts, which means subprojects would
inherit our `CFLAGS="-Wall -Werror"` in their environments.
GNU Make's documentation:
https://www.gnu.org/software/make/manual/html_node/Variables_002fRecursion.html#Variables_002fRecursion
> make exports a variable only if it is either defined in the environment initially...
Example:
```make
A = x
default:
echo $$A
```
then:
```sh
$ make # prints nothing, A is not exported to the subprocess
$ A=y make # prints "x", our A=x is exported to the subprocess
```
Changelog-None
This is clearer, especially when we also deal with raw not-yet-onion-wrapped failure messages. Signed-off-by: Rusty Russell <[email protected]>
1. forward_htlc sets hout to NULL. 2. forward_htlc passes &hout to send_htlc_out. 3. forward_htlc checks the failcode and frees(NULL) and sets hout to NULL (again). This in fact covers every failcode which send_htlc_out returns. We should ensure send_htlc_out sets *houtp to NULL on failure; in fact, both callers pass houtp, so we can make it unconditional. Signed-off-by: Rusty Russell <[email protected]>
For incoming htlcs, we need failure details in case we need to re-xmit them. But for outgoing htlcs, lightningd is telling us it already knows they've failed, so we just need to flag them failed and don't need the details. Internally, we set the ->fail to a dummy non-NULL value; this is cleaned up next. This matters for the next patch, which moves onion handling into lightningd. Signed-off-by: Rusty Russell <[email protected]>
Turn it into temporary node failure: this only happens if we restart with a failed htlc in, but it's clearer and more robust to handle it generically. Signed-off-by: Rusty Russell <[email protected]>
I hadn't realized that lightningd asks gossipd every time we forward a payment. But I'm going to abuse it here to get the latest channel_update, otherwise (as lightningd takes over error message generation) lightningd needs to do an async request at various painful points. So have gossipd tell us the lastest update (stripped so compatible with the strange in-onion-error format). Signed-off-by: Rusty Russell <[email protected]>
Instead of making it ourselves, lightningd does it. Now we only have two cases of failed htlcs: completely malformed (BADONION), and with an already-wrapped onion reply to send. This makes channeld's job much simpler. Signed-off-by: Rusty Russell <[email protected]>
It's only called from the db code, and failing_channel is always NULL. Signed-off-by: Rusty Russell <[email protected]>
We're going to change our internal structure next, so this is preparation. We populate existing errors with temporary node failures, for simplicity. Signed-off-by: Rusty Russell <[email protected]>
We'll use this in the next patch for when we need to create errors to send back to lightningd; most commonly when the channel doesn't have capacity for the HTLC. Signed-off-by: Rusty Russell <[email protected]>
…g HTLCs At the moment, we store e.g. WIRE_TEMPORARY_CHANNEL_FAILURE, and then lightningd has a large demux function which turns that into the correct error message. Such an enum demuxer is an anti-pattern. Instead, store the message directly for output HTLCs; channeld now sends us an error message rather than an error code. For input HTLCs we will still need the failure code if the onion was bad (since we need to prompt channeld to send a completely different message than normal), though we can (and will!) eliminate its use in non-BADONION failure cases. Signed-off-by: Rusty Russell <[email protected]>
We tell channeld that an htlc is bad by sending it a 'struct failed_htlc'. This usually contains an onionreply to forward, but for the case where the onion itself was bad, it contains a failure code instead. This makes the "send a failed_htlc for a bad onion" a completely separate code path, then we can work on removing failcodes from the other path. In several places 'failcode' is now changed to 'badonion' to reflect that it can only be a BADONION failcode. Signed-off-by: Rusty Russell <[email protected]>
… failcode. Unfortunately the invoice_payment_hook can give us a failcode, so I simply restrict it to the two sensible ones. Signed-off-by: Rusty Russell <[email protected]> Changelog-deprecated: plugins: invoice_payment_hook "failure_code" only handles simple cases now, use "failure_message".
…TLC errors (unless BADONION). This cleans up the "local failure" callers for incoming HTLCs to hand an onionreply instead of making us generate it from the code inside make_failmsg. (The db path still needs make_failmsg, so that's next). Signed-off-by: Rusty Russell <[email protected]> Changelog-deprecated: Plugins: htlc_accepted_hook "failure_code" only handles simple cases now, use "failure_message".
…all onionreplyies. This completes the conversion; any in-flight HTLC failures get turned into temporary_node_failures. Signed-off-by: Rusty Russell <[email protected]>
That's all it's used for now. And remove unreferenced failoutchannel. Signed-off-by: Rusty Russell <[email protected]>
As promised in the Changelog when we converted from failcodes to messages internally. Signed-off-by: Rusty Russell <[email protected]>
Changelog-None
Before this patch we used to send `double`s over the wire by just copying them. This is not portable because the internal represenation of a `double` is implementation specific. Instead of this, multiply any floating-point numbers that come from the outside (e.g. JSONs) by 1 million and round them to integers when handling them. * Introduce a new param_millionths() that expects a floating-point number and returns it multipled by 1000000 as an integer. * Replace param_double() and param_percent() with param_millionths() * Previously the riskfactor would be allowed to be negative, which must have been unintentional. This patch changes that to require a non-negative number. Changelog-None
Replace `json_to_double()` (which uses `strtod(3)`) with our own floating-point parsing function `json_to_millionths()` that specifically expects to receive such a number that can fit in a 64 bit integer after being multiplied by 1 million. The main piece of the code in this patch comes from ElementsProject#3535 (comment) Changelog-None
We specify `PYTHON_VERSION=3` to prevent libwally's ./configure from searchin for python2, which some distros have started removing, and we were requiring it only for the configuration step anyway. Changelog-Changed: dependencies: We no longer depend on python2 which has reached end-of-life
We aren't using it and it's broken with the specified upstream image, so remove it outright.
We were waiting for both stdin and stdout to close, however that resulted in us deferring cleanup indefinitely since we did not poll stdout for being writable most of the time. On the other hand we are almost always polling the plugin's stdout, so that notifies us as soon as the plugin stops. Changelog-Fixed: plugin: Plugins no longer linger indefinitely if their process terminates
Changelog-Fixed: plugin: A crashing plugin will no longer cause a hook call to be delayed indefinitely
We make the current state of `lightningd` explicit so we don't have to identify a shutdown by its side-effects. We then use this in order to prevent the killing and freeing of plugins to continue down the chain of registered plugins.
We are attaching the destructor to notify us when the plugin exits, but we also need to clear them once the request is handled correctly, so we don't call the destructor when it exits later.
We promised we'd be waiting up to 20 seconds, but were only waiting for 10. Fix that by bumping to the documented 20.
It was a pointer into the list of plugins for the hook, but it was rather unstable: if a plugin exits after handling the event we could end up skipping a later plugin. We now rely on the much more stable `call_chain` list, so we can clean up that useless field.
As discussed with Christian, prepending the length to the payload returned is awkward, but it's the only way to set a legacy payload. As this will be soon deprecated, simplify the external API. Signed-off-by: Rusty Russell <[email protected]>
Changelog-None
The peer will fail the channel on reconnect if we send an 'add' we don't have balance for yet; we can avoid this issue by always sending fulfills (+) before sending adds (-)
There's a race condition with loading the submodules that's causes a build failure on my machine, since the libwally 'includes' aren't on disk yet when the gcc build step starts.
fixup! docker: Remove Dockerfile for i386 builder
Suggested-by: Matt Whitlock <@whitslack> Signed-off-by: Christian Decker <@cdecker>
We only support a very limited number of argument combinations, and apparently sometimes we trigger a case we aren't handling. This adds a more useful error message, including the params we didn't match.
Changelog-Added: wallet: The wallet now has a gap limit that is used to check for incoming transactions when scanning the blockchain.
Commit 9aedb0c changed this from allocating off `c` to allocating off NULL, knowing that it's tal_steal() in the callback. But before that, it can be detected as a mem leak: ``` @pytest.fixture def teardown_checks(request): """A simple fixture to collect errors during teardown. We need to collect the errors and raise them as the very last step in the fixture tree, otherwise some fixtures may not be cleaned up correctly. Require this fixture in all other fixtures that need to either cleanup before reporting an error or want to add an error that is to be reported. """ errors = TeardownErrors() yield errors if errors.has_errors(): # Format a nice list of everything that went wrong and raise an exception request.node.has_errors = True > raise ValueError(str(errors)) E ValueError: E Node errors: E Global errors: E - Node /tmp/ltests-iz9y1chb/test_hsmtool_secret_decryption_1/lightning-1/ has memory leaks: [ E { E "backtrace": [ E "ccan/ccan/tal/tal.c:442 (tal_alloc_)", E "lightningd/jsonrpc.c:848 (parse_request)", E "lightningd/jsonrpc.c:941 (read_json)", E "ccan/ccan/io/io.c:59 (next_plan)", E "ccan/ccan/io/io.c:407 (do_plan)", E avis/build/ElementsProject/lightning/lightningd/../plugins/pay ``` Reported-by: @niftynei Signed-off-by: Rusty Russell <[email protected]>
We want to compare to `i`, the enum counter, not feerates[i], the
feerate value.
Without this fix, `feerates` appears as:
```
{
"perkw": {
"opening": 253,
"mutual_close": 253,
"unilateral_close": 253,
"delayed_to_us": 253,
"htlc_resolution": 253,
"penalty": 253,
"min_acceptable": 253,
"max_acceptable": 2500,
"min_acceptable": 253,
"max_acceptable": 4294967295,
"urgent": 253,
"normal": 253,
"slow": 506
},
"onchain_fee_estimates": {
"opening_channel_satoshis": 177,
"mutual_close_satoshis": 170,
"unilateral_close_satoshis": 151,
"htlc_timeout_satoshis": 167,
"htlc_success_satoshis": 177
}
}
```
bug introduced in "chaintopology: better feerate targets differentiation"
Changelog-None
Reported-by: SimonVrouwe <[email protected]> Signed-off-by: Antoine Poinsot <[email protected]>
It seems we aren't registering the featurebits correctly.
The documentation was wrong, and I copied my mistake to `libplugin` where it was then ignored instead of ORed into the node's featurebits. This fixes both.
The new `keysend` plugin modifies the node features that we send to peers. This commit breaks out the 'expected_features' we use for tests to encompass this differentiation.
Since the node announcements now include the 55th bit flag for keysends, the total amount of bytes read from disk is now +8
We're doing an RC3 folks!
0.8.2, we're doing it
Changelog-Added: docs: Install documentation now has information about building for Alpine linux
Signed-off-by: Antoine Poinsot <[email protected]>
This in addition removes the init fixed timeout hack. Changelog-fixed: We now *always* die if our Bitcoin backend failed unexpectedly. Signed-off-by: Antoine Poinsot <[email protected]>
They are already logged both in bcli, and in lightningd. This just adds a lot of noise to the logs. We keep successed attempts though for the tests. Signed-off-by: Antoine Poinsot <[email protected]>
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.