We were triggering a second exception in the directory cleanup step by
attempting to access a field that'd only be set upon entering the test code
itself. That error did not contribute to the problem resolution, so now we
check whether that field is set before accessing it.
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Fixes#2518
Signed-off-by: Christian Decker <decker.christian@gmail.com>
Changelog-fixed: `minconf` no longer gets wrapped around for large values, which was causing funds with insufficient confirmations to be selected.
* remove libbase58, use base58 from libwally
This removes libbase58 and uses libwally instead.
It allocates and then frees some memory, we may want to
add a function in wally that doesn't or override
wally_operations to use tal.
Signed-off-by: Lawrence Nahum lawrence@greenaddress.it
Currently, when a multiplication operator is invoked that
does not result in an even integer result but a floating result,
the pylightning code will raise an exception:
Millisatoshi must be string with msat/sat/btc suffix or int
This is because the internal float result will be used as
contructor argument like this: return Millisatoshi(10000.5)
This happens especially on fee calculations where small uneven amounts
are calculated.
If we have plugins configured in the default config location, then
`check-manpages` may fail due to plugin cli options being added to the
`--help` output.
Without this, the connect command hangs in one of my branches. This logic
is from the old days when gossipd handled connections, and we wanted
to make sure it didn't hang up on this client due to the error.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Some tests require dev support, but the rest can run. We simplify
the gossip_store output so it's the same in non-dev mode too.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It can take a while if bitcoind has the regtest chain, and grossly
distorts our benchmarks!
Reported-by: Joe Netti <jnetti@blockstream.io>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
added sanity check to make sure scid of csv is the same as scid in gossip.
Revised style, mem allocation, and error checks
[ Minor fixups, and updated benchmark script -- RR ]
With data.tar.gz: 456609740 Apr 2 12:33
store_load_msec:35300-42354(37118.2+/-2.7e+03)
vsz_kb:582832
store_rewrite_sec:12.700000-13.430000(12.988+/-0.27)
listnodes_sec:3.000000-3.160000(3.076+/-0.057)
listchannels_sec:30.790000-31.690000(31.03+/-0.34)
routing_sec:0.00
peer_write_all_sec:63.640000-67.860000(66.294+/-1.4)
I misunderstood the API, this ended up nesting a result inside the JSON-RPC
result.
No concerns about backwards compatibility since this is so new.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This seems like overkill, at least for now. Handling the JSON
inline is clearer, for the existing examples at least.
We also remove the dummy hook, rather than fix it.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Now we can benchmark, and remove 500 bytes per node.
MCP results from 5 runs, min-max(mean +/- stddev):
store_load_msec:35093-37907(36146+/-1.1e+03)
vsz_kb:555168
store_rewrite_sec:12.120000-13.750000(12.7+/-0.6)
listnodes_sec:1.270000-1.370000(1.322+/-0.039)
listchannels_sec:29.770000-31.600000(30.82+/-0.64)
routing_sec:0.00
peer_write_all_sec:63.630000-67.850000(65.432+/-1.7)
MCP notable changes from pre-Dijkstra (>1 stddev):
-vsz_kb:577456
+vsz_kb:555168
-routing_sec:60.70
+routing_sec:12.04
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Do it inside the can_reach() function, which is less optimal for BFG
which does 20 ops on the same channel, but fine for Dijkstra.
This does have a measurable cost, so we might want to use
non-cryptographic fuzz in future:
$ gossipd/test/run-bench-find_route 100000 100:
Before:
100 (100 succeeded) routes in 100000 nodes in 97346 msec (973461784 nanoseconds per route)
After:
100 (100 succeeded) routes in 100000 nodes in 113381 msec (1133813412 nanoseconds per route)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If a route is too long, we try to bias Dijkstra towards choosing a
shorter route by adding a per-hop cost. We do a naive "shortest path"
pass, then using that cost as a ceiling on per-hop cost, we do a
binary search.
There are some subtleties: we use risk rather than total as our
counter field (we normally bias this by 1 anyway, so it's easy to make
that a variable), and we set riskfactor to a mimimal value once we're
iterating. It's good enough to get a solution, we don't need to do a
2-dimensional search on riskfactor and riskbias.
Of course, this is extremely slow if we hit it on our benchmark,
though it doesn't happen in a more realistic network:
$ gossipd/test/run-bench-find_route 100000 100:
Before:
100 (79 succeeded) routes in 100000 nodes in 25341 msec (253412314 nanoseconds per route)
After:
100 (100 succeeded) routes in 100000 nodes in 97346 msec (973461784 nanoseconds per route)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Our uintmap can be a little slow with all the reallocation, so leave
NULL entries and walk to find the first one. Since we don't clean
them up, keep a cache of where the min non-all-NULL value is in the
heap.
It's clearer benefit on really large tests, so here's 1M nodes:
Comparison using gossipd/test/run-bench-find_route 1000000 10:
Before:
10 (10 succeeded) routes in 1000000 nodes in 91995 msec (9199532898 nanoseconds per route)
After:
10 (10 succeeded) routes in 1000000 nodes in 20605 msec (2060539287 nanoseconds per route)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Use a uintmap as our minheap.
Note that Dijkstra can give overlength routes, so some checks are disabled.
Comparison using gossipd/test/run-bench-find_route 100000 10:
Before:
10 (10 succeeded) routes in 100000 nodes in 120087 msec (12008708402 nanoseconds per route)
After:
10 (10 succeeded) routes in 100000 nodes in 2269 msec (226925462 nanoseconds per route)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This is a weakness with Dijkstra, so write an explicit unit test that
we can find a short enough (but more expensive) route.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Channels have a htlc_minimum_msat of 10000, which is why we didn't
find routes.
This makes a significant speed drop:
-routing_sec:26.940000-27.990000(27.616+/-0.39)
+routing_sec:60.70
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
68fe5eacde introduced a skip in the iteration
of the available funds, which means utxos[i] may be off the end of utxos.
Reported-and-debugged-by: @nitramiz
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
For me this happened only under valgrind with
test_option_upfront_shutdown_script (in a pending branch):
==5063== by 0x51FC076: raise (raise.c:48)
==5063== by 0x51DD534: abort (abort.c:79)
==5063== by 0x1292D2: fatal (log.c:647)
==5063== by 0x116570: channel_set_state (channel.c:340)
==5063== by 0x116E04: lockin_complete (channel_control.c:73)
==5063== by 0x116F15: peer_got_funding_locked (channel_control.c:108)
==5063== by 0x117354: channel_msg (channel_control.c:208)
No CHANGELOG: this was introduced in a recent refactor.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We generated blocks to announce the channel, but it can also expire
the HTLC if the timing is wrong. We don't need to anyway, since we
fixed the FIXME; we store local unannounced channels for restoration
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Millisatoshi's inner representation expected to be an int, otherwise unwanted exceptions could occur
The following example raises: TypeError: __int__ returned non-int (type decimal.Decimal)
from lightning import Millisatoshi
one_sat = Millisatoshi("1sat")
two_sats = one_sat * 2
The old value of 1000 sat was too small to cover the dust reserves.
This lead to the situation when trying to open a channel with minimal
amount, the channels got refused because they were not able cover the
commitment fees.
For this reason the minimal capacity should be increased to i.e. 10k
satoshi, as the technical minimum that also accounts for fees and
reserves is somewhere around 6k sat.
The spec says not to send a commitment_signed without any changes, but LND
does this. To understand why, you have to understand how LND works. I
haven't read the code, but I'm pretty sure it works like this:
1. lnd slows down to do garbage collection, because it's in Go.
2. When an alert timer goes off, noticing it's not making process, it
sends a twitter message to @roasbeef.
3. @roasbeef sshs into the user's machine and binary patches lnd to send
a commitment_signed message.
4. Unfortunately he works so fast that various laws of causality are broken,
meaning sometimes the commitment_signed is sent before any of thes
other things happen.
I'm fairly sure that this will stop as @roasbeef ages, or lnd introduces
some kind of causality enforcement fix.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Refactored the weighted-reservoir-sampling algo to make it more straightforward.
It now uses the excess as fraction of capacity as weight. This favors channels that
are more _relatively_ unbalanced to be used for incoming payment.
Now passes test_invoice_routeboost_private() when using max fundamount=16777215.
Christian Decker
GreenAddress
arowser
Michael Schmoock
Rusty Russell
Joe Netti
lisa neigut
Hampus Sjöberg
Lawrence Nahum
Gálli Zoltán
Simon Vrouwe
trueptolemy