We make it a first-class citizen internally, even though we won't use
it over the wire (at least, non-experimental builds). This scheme
follows the latest draft, in which features are flagged compulsory.
We also add several helper functions.
Since uses the *even* bits (as per latest spec), not the *odd* bits,
we have some other fixups.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We want to use this to handle the simple description for channel_type.
It also needs to handle variable-size types (just like subtypes).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Updates from output of latest version of shellcheck as per SC2268
“Avoid x-prefix in comparisons as it no longer serves a purpose”
https://www.shellcheck.net/wiki/SC2268
1. listpeers has a deprecated `"closer": null`, which we need
to handle in the schema, while trying not to damage our
documentation too much.
2. Don't print a condition if there are no fields to print.
3. Allow a special "untyped" marker for multifundchannel which returns
arbitrary JSON in a field.
4. Allow a single field return (for 'stop').
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I did this by copying the updated bech32 code, and then re-patching in
our minor changes:
1. Headers modded (we need size_t)
2. Explicit length for bech32_encode/decode (not 90).
3. Exposing and bech32_ prefix for convert_bits, charset, charset_rev.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This matters for:
- common/wallet.h vs wallet/wallet.h
- common/gossip_store.h vs gossipd/gossip_store.h
- common/json.h vs lightningd/json.h
- common/ping.h vs lightningd/ping.h
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
If you do update-mocks in a dirty tree, the recursive make that it
uses will try to rebuild things! Suppress that.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We should actually be including this (as it may define _GNU_SOURCE
etc) before any system headers. But where we include <assert.h> we
often didn't, because check-includes would complain that the headers
included it too.
Weaken that check, and include config.h in C files before assert.h.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Sometimes it's v0.9.3 and sometimes it's 0.9.3, and I always get it
wrong. This is painful when you have to create a signed tag after
committing to one of the two only to find out you chose wrong and have
to redo the tagging.
This would cause a segfault on the default network parameter for
`dumponchaindescriptors`.
Introduced in 1513a2d07e
Changelog-Fixed: hsmtool: fix a segfault on `dumponchaindescriptors` without network parameter
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
This makes use of the constant defined in the previous commits to more
accurately detect plaintext, encrypted, and invalid seeds. We now error
on invalid seeds.
Changelog-changed: hsmd: we now error at startup on invalid hsm_secret
Changelog-changed: hsmtool: all commands now error on invalid hsm_secret
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
This avoids duplication of both logic and error-prone values, such as
the salt. Grouping all hsm encryption logic into a public API will also
allow us to fuzz it.
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
Changelog-changed: lightningd: the `--encrypted-hsm` now asks you to confirm your password when first set
Changelog-changed: hsmtool: the `encrypt` now asks you to confirm your password
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
Fixes#4302
Changelog-fixed: hsmtool: the `generatehsm` command now generates an appropriately-sized hsm_secret
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
This slightly breaks the API, but still accept the input: we just don't
take it into account anymore.
For `dumponchaindescriptors`, we have to still take the old place of the
`network` parameter into account to not entirely break the API.
Changelog-Added: hsmtool: password must now be entered on stdin. Password passed on the command line are discarded.
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
This is vital for calculating merkle trees; I previously used
towire+fromwire to get this!
Requires generation change so we can magic the ARRAY_SIZE var (the C
pre-processor can't uppercase things).
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Actually, it's more complex to translate the xpub descriptor to
testnet because of the descriptor checksum.
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
This adds a command which outputs the two output descriptors
corresponding to our onchain wallet.
This can be useful for an external service to monitor / send fund to our
wallet.
Further, an "xpriv" version of such descriptors could be used to import
onchain funds on a new wallet.
Changelog-Added: lightning-hsmtool: a new command was added to hsmtool for dumping descriptors of the onchain wallet
Signed-off-by: Antoine Poinsot <darosior@protonmail.com>
tools: Add `generatehsm` method to hsmtool to derivate BIP32 seeds from a
mnemonic using the BIP39 standard.
The new method uses libwally for the BIP39 to BIP32 derivation. It also
fails if an hsm_secret file already exists, so we do not overwrite
someone else's wallet without noticing.
It allows the use of passphrases, the ECHO mode in the terminal is
disable for higher security.
It currently supports "en", "es", "fr", "it", "jp", "zhs", "zht".
Changelog-Added: hsmtool: `hsm_secret` generation from a seed-phrase following BIP39.
Note that check-whitespace and check-bolt already do this, so we
can eliminate redundant lines in common/Makefile and bitcoin/Makefile.
We also include the plugin headers in ALL_C_HEADERS so they get
checked.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
There's a lot of it, and it means we can't `make check-source` on
these files.
Also bring bolt quotes up-to-date.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This avoids overwriting the ones in git, and generally makes things neater.
We have convenience headers wire/peer_wire.h and wire/onion_wire.h to
avoid most #ifdefs: simply include those.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We have to handle singletons which are arrays of variable-length entries:
this needs to be a ptr-to-ptr.
```C
struct blinded_payinfo {
u32 fee_base_msat;
u32 fee_proportional_millionths;
u16 cltv_expiry_delta;
u8 *features;
};
```
Before:
```C
struct tlv_invoice_tlvs {
...
struct blinded_payinfo *blindedpay;
```
After:
```C
struct tlv_invoice_tlvs {
...
struct blinded_payinfo **blindedpay;
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
has_len_fields() doesn't cover our blacklist of variable types, so if
we have an array of them, this logic is wrong. This happens in the
the bolt13 patch:
```C
struct tlv_offer_tlvs_blindedpath {
struct pubkey blinding;
struct onionmsg_path **path;
};
```
Before:
wire/gen_bolt13_tlv.c:
```C
for (size_t i = 0; i < tal_count(r->blindedpath->path); i++)
towire_onionmsg_path(&ptr, r->blindedpath->path + i);
```
After:
```C
for (size_t i = 0; i < tal_count(r->blindedpath->path); i++)
towire_onionmsg_path(&ptr, r->blindedpath->path[i]);
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Otherwise make thinks we're done, and we can get errors. Include
primitive code if we abort build halfway and leave .refresh-submodules dir.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We create ALL_PROGRAMS, ALL_TEST_PROGRAMS, ALL_C_SOURCES and
ALL_C_HEADERS. Then the toplevel Makefile knows which are
autogenerated (by wildcard), so it can have all the rules to clean
them or check the source as necessary.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Fixed: Actually make the password argument optional for `guesstoremote` and `dumpcommitments` sub-commands, as shown in our documentation and help text.
Reported by `Barno` on #c-lightning IRC.
Before this, docker image will never detects that
`lightning-rpc` was created if it is running in regtest
or testnet, because the file will be created under
subfolder for each network name, and entrypoint does not
check "lightning-rpc" file in those folders.
By specifying `LIGHTNINGD_NETWORK` environment var
in dockerfile, we can now check correct path.
Changelog-Added: Docker build now includes `LIGHTNINGD_NETWORK` ENV variable which defaults to "bitcoin". An user can override this (e.g. by `-e` option in `docker run`) to run docker container in regtest or testnet or any valid argument to `--network`.
Changelog-Added: We now install `lightning-hsmtool` for your `hsm_secret` needs.
See: https://github.com/ElementsProject/lightning/issues/3717#issuecomment-644844594
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.
According to #3226, it looks like clang's LD error format has changed.
This patch adds the new format so we can parse the mocks successfully.
Apple clang version 11.0.3 (clang-1103.0.32.29)
```
checking for ANSI C header files... Undefined symbols for architecture x86_64:
"_fromwire_amount_msat", referenced from:
_fromwire_tlv_test_n1_tlv3 in ccj4zKdV.o
"_fromwire_bool", referenced from:
_fromwire_test_msg in ccj4zKdV.o
_fromwire_test_msg_option_short_id in ccj4zKdV.o
_fromwire_test_msg_option_one in ccj4zKdV.o
_fromwire_test_msg_option_two in ccj4zKdV.o
"_fromwire_test_enum", referenced from:
_fromwire_test_msg in ccj4zKdV.o
_fromwire_test_msg_option_short_id in ccj4zKdV.o
_fromwire_test_msg_option_one in ccj4zKdV.o
_fromwire_test_msg_option_two in ccj4zKdV.o
"_fromwire_tlvs", referenced from:
_fromwire_test_tlv1 in ccj4zKdV.o
_fromwire_test_tlv2 in ccj4zKdV.o
_fromwire_test_tlv3 in ccj4zKdV.o
"_fromwire_tu32", referenced from:
_fromwire_tlv_test_n2_tlv2 in ccj4zKdV.o
"_fromwire_tu64", referenced from:
_fromwire_tlv_test_n1_tlv1 in ccj4zKdV.o
_fromwire_tlv_test_n2_tlv1 in ccj4zKdV.o
"_fromwire_u16", referenced from:
_fromwire_test_features in ccj4zKdV.o
_fromwire_subtype_var_assign in ccj4zKdV.o
_fromwire_subtype_arrays in ccj4zKdV.o
_fromwire_tlv_test_n1_tlv4 in ccj4zKdV.o
_fromwire_tlv_test_n3_tlv3 in ccj4zKdV.o
_fromwire_test_msg in ccj4zKdV.o
_fromwire_test_tlv1 in ccj4zKdV.o
...
"_fromwire_u32", referenced from:
_fromwire_tlv_test_n3_tlv3 in ccj4zKdV.o
_fromwire_test_msg in ccj4zKdV.o
_fromwire_test_msg_option_short_id in ccj4zKdV.o
_fromwire_test_msg_option_one in ccj4zKdV.o
_fromwire_test_msg_option_two in ccj4zKdV.o
"_fromwire_u64", referenced from:
_fromwire_test_short_id in ccj4zKdV.o
_fromwire_tlv_test_n3_tlv3 in ccj4zKdV.o
"_fromwire_u8", referenced from:
_fromwire_subtype_var_assign in ccj4zKdV.o
_fromwire_subtype_var_len in ccj4zKdV.o
_fromwire_subtype_varlen_varsize in ccj4zKdV.o
_fromwire_tlv_test_n3_tlv3 in ccj4zKdV.o
"_fromwire_u8_array", referenced from:
_fromwire_test_features in ccj4zKdV.o
_fromwire_subtype_arrays in ccj4zKdV.o
_fromwire_tlv_test_n3_tlv3 in ccj4zKdV.o
_fromwire_test_msg in ccj4zKdV.o
_fromwire_test_msg_option_short_id in ccj4zKdV.o
_fromwire_test_msg_option_one in ccj4zKdV.o
_fromwire_test_msg_option_two in ccj4zKdV.o
...
```
Changelog-Fixed: Build for macOS Catalina / Apple clang v11.0.3 fixed
`struct tx_parts` is just a txid and a bunch of inputs and outputs,
some of which may be NULL.
This is both a nod towards a future where we (or our peer) can combine
HTLCs or (in an eltoo world) commitments, although for the moment all
our tx_parts will be complete.
It also matches our plan to split `bitcoin_tx` into two types: this
`struct tx_parts` where we don't know input amounts etc, and `psbt`
where we do.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
now that witness script data is saved into the tx/psbt which is
serialized across the wire, there's no reason to use witscript to do
this. good bye witscript!
- we've moved tmpctx management to setup.c from daemon.c, so we update
the `check-tmpctx`
- `common_setup(char *)` is now a valid analog for `setup_locale`, so we
check for either in check-setup_locale
The formatting makes it harder for update-mocks, eg:
/* Generated stub for fmt_wireaddr_without_port */
char *fmt_wireaddr_without_port(const tal_t *ctx UNNEEDED, const struct wireaddr *a UNNEEDED)
{ fprintf(stderr, "fmt_wireaddr_without_port called!\n"); abort(); }
/* Could not find declaration for fromwire_onionmsg_path */
/* Generated stub for json_add_member */
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
When we have only a single member in a TLV (e.g. an optional u64),
wrapping it in a struct is awkward. This changes it to directly
access those fields.
This is not only more elegant (60 fewer lines), it would also be
more cache friendly. That's right: cache hot singles!
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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.
This will be used when we want to specify these in a route. But for now, they
only alter gossipd, which always sets them to NULL.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Use `LC_ALL=C sort` instead of `sort` so that mocks get sorted in
the same way on all developers' environments.
Re-record the result of `make update-mocks`.
Changelog-None
We currently abuse the added_htlc and failed_htlc messages to tell channeld
about existing htlcs when it restarts. It's clearer to have an explicit
'existing_htlc' type which contains all the information for this case.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
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
Before this patch we used `int` for error codes. The problem with
`int` is that we try to pass it to/from wire and the size of `int` is
not defined by the standard. So a sender with 4-byte `int` would write
4 bytes to the wire and a receiver with 2-byte `int` (for example) would
read just 2 bytes from the wire.
To resolve this:
* Introduce an error code type with a known size:
`typedef s32 errcode_t`.
* Change all error code macros to constants of type `errcode_t`.
Constants also play better with gdb - it would visualize the name of
the constant instead of the numeric value.
* Change all functions that take error codes to take the new type
`errcode_t` instead of `int`.
* Introduce towire / fromwire functions to send / receive the newly added
type `errcode_t` and use it instead of `towire_int()`.
In addition:
* Remove the now unneeded `towire_int()`.
* Replace a hardcoded error code `-2` with a new constant
`INVOICE_EXPIRED_DURING_WAIT` (903).
Changelog-Changed: The waitinvoice command would now return error code 903 to designate that the invoice expired during wait, instead of the previous -2
We cannot let users use `sendcustommsg` to inject messages that are handled
internally since it could result in our internal state tracking being borked.
I really want a type which means "I am a wrapped onion reply" as separate
from "I am a normal wire msg". Currently both user u8 *, and I got very
confused trying to figure out where each one was an unwrapped error msg,
or where it still needed (un)wrapping.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Add towire_int() and fromwire_int() functions to "(de)serialize"
"int". This will only work as long as both the caller of towire_int()
and the caller of fromwire_int() use the same in-memory representation
of signed integers and have the same sizeof(int).
Changelog-None
TIL: `rename` doesn't like its source and target to be on different
partitions. This was causing the `hsmtool` tests to fail whenever we ran them
on a different partition than the lightning-dir (e.g., `/dev/shm` for faster
testing), because we made the backup copy in the current working directory.
This changes this and creates the backup next to the original file, which has
a reasonable chance to be on the same partition.
Changelog-Changed: hsmtool: The `hsmtool` now creates its backup copy in the same directory as the original `hsm_secret` file.
Recent FreeBSD versions use LLVM's lld as a linker, not the GNU ld.
Their behavior slightly differs, so adapt the build system to handle
either one.
* The LLVM's linker prints "undefined symbol:" instead of
"undefined reference to". Tweak tools/mockup.sh to also look for that
message.
* The LLVM's linker may only print the first dozen errors (omitting
the rest to avoid flooding the screen). tools/update-mocks.sh relies
on getting all errors as it extracts the missing symbols' names from
the error output and creates mocks for them. Detect if errors were
omitted and re-run, telling the linker to not omit any. The GNU linker
does not support -error-limit=0, so unfortunately we can't just run
with that option unconditionally from the first attempt.
* Nit: FreeBSD's sed(1) prints "t" for "\t" instead of a horizontal tab.
Use a verbatim tab in the command, instead of "\t" which works on
both.
Changelog-Fixed: Developer mode compilation on FreeBSD.
This is the final step: we pass the complete fee_states to and from
channeld.
Changelog-Fixed: "Bad commitment signature" closing channels when we sent back-to-back update_fee messages across multiple reconnects.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This, in the case of data loss on a channel with `option_static_remotekey`
negotiated, allows to likely (if the dbid is not unreasonable) recover
the funds from a remote unilateral close just with the hsm_secret.
Changelog-added: A new command, 'guesstoremote', is added to the hsmtool. It is meant to be used to recover funds after an unilateral close of a channel with `option_static_remotekey` enabled.
Since the parser itself just parses and doesn't include validation anymore we
need to put that functionality somewhere. The validation consists of enforcing
that the types are in monotonically increasing order without duplicates and
that for the even types we know how to handle it.
We were weaving in and out of generic code through `fromwire_tlvs` with custom
parameters defining the types in that namespace. This hard-wires the parser
with the namespace's types. Slowly trying to deprecate `fromwire_tlvs` in
favor of this typesafe variant.
This takes a dbid, a "depth" (how many points to dump), the hsm_secret
path, and a potential password to dump informations about all
commitments until the depth.
Co-Authored-By: Sjors Provoost <sjors@sprovoost.nl>
We now have a pointer to chainparams, that fails valgrind if we do anything
chain-specific before setting it.
Suggested-by: Rusty Russell <@rustyrussell>
These are special in extended regexs, and so we fail to match once we fix
the BOLT comment in common/test/run-bigsize.c
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Somehow this change got lost, but it's needed for option_static_remotekey,
to quote gen_peer_wire_csv:
msgtype,channel_reestablish,136
msgdata,channel_reestablish,channel_id,channel_id,
msgdata,channel_reestablish,next_commitment_number,u64,
msgdata,channel_reestablish,next_revocation_number,u64,
msgdata,channel_reestablish,your_last_per_commitment_secret,byte,32,option_data_loss_protect,option_static_remotekey
msgdata,channel_reestablish,my_current_per_commitment_point,point,,option_data_loss_protect
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Makefile:424: warning: overriding recipe for target 'clean'
tools/Makefile:12: warning: ignoring old recipe for target 'clean'
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Because it required update-mocks, which is a hack which relies on the
format of linker errors (!) I'd prefer to make this --enable-developer
only.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We need to tell it that the tmp file is an intermediate, so doesn't need
remaking if it doesn't exist.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Our TLV serializer relies on TLV outputs to be ordered by type
number. Prior to this commit we relied on 1) the ordering in the
RFC to be correct and 2) users to be using a version of Python that
respects stable ordering of dicts (i.e. Python 3.7+)
Instead of relying on these implicitly, we now explicitly sort messages
by type number when the TLV sets.
Resolves#2956.
Thanks-To: @ScottTre for the sort function
Reported-By: @ZmnSCPxj
1. These days we delete the [Unreleased] tag during rcs.
2. Make sure we test the release build process during rc1, since I
screwed that up last release.
3. Add a section on rc2, etc.
4. Do final release via a github PR, since I screwed that up on the
prior release.
5. Update `tools/build-release.sh` and instructions to show that we now
make a reproducible build for Ubuntu 18.04 x86-64.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This simplifies the dependencies:
1. Objs depend on headers, not other objs.
2. Programs depend on objs.
3. A .o file will generally implicitly depend on the .c file it's built from.
4. If a file has a build line, it's often better to list all deps there.
5. I spotted some missing 'make clean' files.
The particular problem in this case seems to be that make would use
tools/test/gen_test.c before it was ready. It's probably confused by
the use of recursive make via update-mocks, so explicitly split that
into two stages.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Naturally, it's a struct pubkey. However, those are large, and take
time to marshal, so gossipd treats them as node_id which is a simple
array. It adds explicit checks at the right points to make sure
they're valid pubkeys.
However, the next commit adds TLV test vectors, which assumes we treat
node_id as a point (thus catch invalid values when parsing). The best
solution is to restrain our types here to exactly those we've
optimized for.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This makes it build for me on FreeBSD 11:
1. $(MAKE) has to passed through into update-mocks.
2. FreeBSD sed doesn't turn \n into a newline on RHS.
3. Bash and mako dependencies were missing from INSTALL.md
Fixes: #2850
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
TLVs have an implicit `len` field, so allow expressions containing
that (eg. `len-1`), but assume it means "the remainder of the
message".
This means in most places, f.size() needs an fallback for the
implicit-length case.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
They're currently called varint, but there's a proposal to call them all
bigsize. Allow both for now.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We need to hand -s to both header and body generation, or neither:
wire/gen_peer_wire.c:53:13: error: static declaration of ‘towire_channel_update_timestamps’ follows non-static declaration
In file included from wire/gen_peer_wire.c:5:
./wire/gen_peer_wire.h:78:6: note: previous declaration of ‘towire_channel_update_timestamps’ was here
We also need it for printwire, otherwise we get static unused functions for subtypes:
devtools/gen_print_wire.c:155:13: error: ‘printwire_channel_update_checksums’ defined but not used [-Werror=unused-function]
static void printwire_channel_update_checksums(const char *fieldname, const u8 **cursor, size_t *plen)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
devtools/gen_print_wire.c:133:13: error: ‘printwire_channel_update_timestamps’ defined but not used [-Werror=unused-function]
static void printwire_channel_update_timestamps(const char *fieldname, const u8 **cursor, size_t *plen)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
```
tools/test/enum.c: In function ‘fromwire_test_enum’:
tools/test/enum.c:11:34: error: format ‘%ld’ expects argument of type ‘long int’, but argument 2 has type ‘size_t {aka unsigned int}’ [-Werror=format=]
printf("fromwire_test_enum at %ld\n", *max);
```
and:
```
devtools/print_wire.c: In function ‘printwire_tlvs’:
devtools/print_wire.c:201:22: error: format ‘%ld’ expects argument of type ‘long int’, but argument 2 has type ‘u64 {aka long long unsigned int}’ [-Werror=format=]
printf("**TYPE #%ld UNKNOWN for TLV %s**\n", type, fieldname);
^
```
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
originally done so that any field within a subtype needed context,
that would be reflected at the top-most layer. in reality, we
allocate subtype fields off of the instance of the subtype, so
there's no need to check beneath the 'top' layer of field/types
in a message to determine whether or not to pass in a context.
Add parsing know-how for enum fields. This is necessary for
internally defined wire generators. Enums are denoted by prefixing
the field with an `e:`.
Ex:
msgdata,msg_name,field_name,e:enum_type,
we rely, perhaps a bit hackily, on there only being one copy of
each type object floating about. using `deepcopy` on a Message
for message extensions destroys this paradigm, which breaks
things in the case where it's a later defined subtype that contains
variable-length members.
to fix this, we modify `__deepcopy__` on the Field class, such
that it preserves the reference to the original type_obj instance.
remove extra space from the lead-in for inline comments
so that a provided comment like this
# This is a comment
will appear like this
/* This is a comment */
Add a test for checking that the bolt-gens do the right thing
for a fairly exhaustive test case set (and that it compiles).
Note that this doesn't check that we've got the memory assignment
pieces worked out.
It's got a kind of exotic reliance on the update-mocks in that in
order to depend on as little of the wire/ code as possible (we
only import wire/wire.h), we include an AUTOGENERATE comment
in the test_cases CSV file, and then run update-mocks as part of
the build for that file.
we'll need this for internal wire message formats. also disambiguates
from 'bolt message optional fields', which we rename to extensions here.
example of an optional field declaration (note the ? prefixing the
type):
msgdata,msg_name,field_name,?type,count
these are handled with either a boolean if they're not present,
or a true value and then the object if they are.
if there are any comments that aren't "attached" to a message,
print them at the top of the generated file. we need this for
the fancy auto-gen'd dependencies in the tool-wiregen tests.
The new TLV spec uses BigSize, like Bitcoin's CompactInt but
*little-endian*. So change our name for clarity, and insist that
decoding be minimal as the spec requires.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
the RFC's extract-format.py is switching to a new format.
this script can correctly parse them.
mostly moves logic over from generate-wire.py, uses a
Python formatting libarary called mako, which needs to be
installed prior to running this script.
you can add it to your system with
sudo apt-get install python3-mako
Encapsulating the peer state was a win for lightningd; not surprisingly,
it's even more of a win for the other daemons, especially as we want
to add a little gossip information.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We now have a test blockchain for MCP which has the correct channels,
so this is not needed.
Also fix a benchmark script bug where 'mv "$DIR"/log
"$DIR"/log.old.$$' would fail if you log didn't exist from a previous run.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Save some overhead, plus gets us ready for giving subdaemons direct
store access. This is the first time we *upgrade* the gossip_store,
rather than just discarding.
The downside is that we need to add an extra message after each
channel_announcement, containing the channel capacity.
After:
store_load_msec:28337-30288(28975+/-7.4e+02)
vsz_kb:582304-582316(582306+/-4.8)
store_rewrite_sec:11.240000-11.800000(11.55+/-0.21)
listnodes_sec:1.800000-1.880000(1.84+/-0.028)
listchannels_sec:22.690000-26.260000(23.878+/-1.3)
routing_sec:2.280000-9.570000(6.842+/-2.8)
peer_write_all_sec:48.160000-51.480000(49.608+/-1.1)
Differences:
-vsz_kb:582320
+vsz_kb:582316
-listnodes_sec:2.100000-2.170000(2.118+/-0.026)
+listnodes_sec:1.800000-1.880000(1.84+/-0.028)
-peer_write_all_sec:51.600000-52.550000(52.188+/-0.34)
+peer_write_all_sec:48.160000-51.480000(49.608+/-1.1)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
* 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
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)
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>
now we print the subtypes out when you call printwire
note that we have to reverse the order the subtypes appear in
because
1) they're static and,
2) a few of them are nested
the original version of the subtype generator emitted '$'
to designate that a field was a subtype; now it's got a different
format:
funding_type,8,num_inputs,2
funding_type,10,input_info,num_inputs*input_info
this patch updates our generator to understand this new format
This is needed so that some csv's can expose their subtype parsing
functions in their header. This gets used in a later PR where
we start replacing manually created 'subtype' definitions with
generated ones.
This doesn't result in a speedup for our benchmark, since we use the
cli tool which does the formatting.
MCP results from 5 runs, min-max(mean +/- stddev):
store_load_msec:33422-36830(35196.2+/-1.2e+03)
vsz_kb:2637488
store_rewrite_sec:36.030000-37.630000(36.794+/-0.52)
listnodes_sec:0.720000-0.950000(0.86+/-0.077)
listchannels_sec:40.300000-41.080000(40.668+/-0.29)
routing_sec:30.440000-31.030000(30.69+/-0.2)
peer_write_all_sec:50.060000-52.800000(51.416+/-0.91)
MCP notable changes from 2 patches ago (>1 stddev):
-listchannels_sec:48.560000-55.680000(52.642+/-2.7)
+listchannels_sec:40.300000-41.080000(40.668+/-0.29)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Plugins don't do it right anyway, and we're about to remove it from
lightningd. Produces same format as json_pp.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
We can save significant space by combining both sides: so much that we
can reduce the WIRE_LEN_LIMIT to something sane again.
MCP results from 5 runs, min-max(mean +/- stddev):
store_load_msec:34467-36764(35517.8+/-7.7e+02)
vsz_kb:2637488
store_rewrite_sec:35.310000-36.580000(35.816+/-0.44)
listnodes_sec:1.140000-2.780000(1.596+/-0.6)
listchannels_sec:55.390000-58.110000(56.998+/-0.99)
routing_sec:30.330000-30.920000(30.642+/-0.19)
peer_write_all_sec:50.640000-53.360000(51.822+/-0.91)
MCP notable changes from previous patch (>1 stddev):
-store_rewrite_sec:34.720000-35.130000(34.94+/-0.14)
+store_rewrite_sec:35.310000-36.580000(35.816+/-0.44)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Don't turn them to/from pubkeys implicitly. This means nodeids in the store
don't get converted, but bitcoin keys still do.
MCP results from 5 runs, min-max(mean +/- stddev):
store_load_msec:33934-35251(34531.4+/-5e+02)
vsz_kb:2637488
store_rewrite_sec:34.720000-35.130000(34.94+/-0.14)
listnodes_sec:1.020000-1.290000(1.146+/-0.086)
listchannels_sec:51.110000-58.240000(54.826+/-2.5)
routing_sec:30.000000-33.320000(30.726+/-1.3)
peer_write_all_sec:50.370000-52.970000(51.646+/-1.1)
MCP notable changes from previous patch (>1 stddev):
-store_load_msec:46184-47474(46673.4+/-4.5e+02)
+store_load_msec:33934-35251(34531.4+/-5e+02)
-vsz_kb:2638880
+vsz_kb:2637488
-store_rewrite_sec:46.750000-48.280000(47.512+/-0.51)
+store_rewrite_sec:34.720000-35.130000(34.94+/-0.14)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Outputs CSV. We add some stats for load times in developer mode, so we can
easily read them out.
peer_read_all_sec doesn't work, since we seem to reject about half the
updates for having bad signatures. It's also very slow...
routing fails, for unknown reasons, so that failure is ignored in routing_sec.
Results from 5 runs, min-max(mean +/- stddev):
store_load_msec,vsz_kb,store_rewrite_sec,listnodes_sec,listchannels_sec,routing_sec,peer_write_all_sec
39275-44779(40466.8+/-2.2e+03),2899248,41.010000-44.970000(41.972+/-1.5),2.280000-2.350000(2.304+/-0.025),49.770000-63.390000(59.178+/-5),33.310000-34.260000(33.62+/-0.35),42.100000-44.080000(43.082+/-0.67)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Header from folded patch 'fixup!_tools-bench-gossipd.sh__rough_benchmark_for_gossipd_and_the_million_channels_project-2.patch':
fixup! tools/bench-gossipd.sh: rough benchmark for gossipd and the million channels project
Suggested-by: @niftynei
Header from folded patch 'fixup!_tools-bench-gossipd.sh__rough_benchmark_for_gossipd_and_the_million_channels_project-1.patch':
fixup! tools/bench-gossipd.sh: rough benchmark for gossipd and the million channels project
MCP filename change.
Header from folded patch 'tools-bench-gossipd.sh__dont_print_csv_by_default.patch':
tools/bench-gossipd.sh: don't print CSV by default.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Header from folded patch 'fixup!_tools-bench-gossipd.sh__rough_benchmark_for_gossipd_and_the_million_channels_project.patch':
fixup! tools/bench-gossipd.sh: rough benchmark for gossipd and the million channels project
Make shellcheck happy.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
fixup printing methods in devtools/decodemsg such that TLV's can
now be printed as well. here's how you'd use it:
$ ./devtools/decodemsg --tlv opening_tlv 0120001E020202020202020202020202020202020202020202020202020202020202
> WIRE_OPTION_UPFRONT_SHUTDOWN_SCRIPT (size 32):
> shutdown_scriptpubkey=[020202020202020202020202020202020202020202020202020202020202]
TLV's use var_int's for messages sizes, both internally and
in the top level (you should really stack a var_int inside a var_int!!)
this updates our automagick generator code to understand 'var_ints'
passing back a null TLV was crashing here, because we tried to
dereference a null pointer. instead, we put it into a temporary
struct that we can check for NULL-ness, before assigning to the
passed in pointer.
let's let the fromwire__tlv methods allocate the tlv-objects and
return them. we also want to initialize all of their underlying
messages to NULL, and fail if we discover a duplicate mesage type.
if parsing fails, instead of returning a struct we return NULL.
Suggested-By: @rustyrussell
Since messages in TLV's are optional, the ideal way to deal with
them is to have a 'master struct' object for every defined tlv, where
the presence or lack of a field can be determined via the presence
(or lack thereof) of a struct for each of the optional message
types.
In order to do this, appropriately, we need a struct for every
TLV message. The next commit will make use of these.
Note that right now TLV message structs aren't namespaced to the
TLV they belong to, so there's the potential for collision. This
should be fixed when/where it occurs (should fail to compile).
Add tlv-messages to the general messages set so that their parsing
messages get printed out.
FIXME: figure out how to account for partial message length processing?
Version 1.1 of the lightning-rfc spec introduces TLVs for optional
data fields. This starts the process of updating our auto-gen'd
wireformat parsers to be able to understand TLV fields.
The general way to declare a new TLV field is to add a '+' to the
end of the fieldname. All field type declarations for that TLV set
should be added to a file in the same directory by the name
`gen_<field_name>_csv`.
Note that the FIXME included in this commit is difficult to fix, as
we currently pass in the csv files via stdin (so there's no easy
way to ascertain the originating directory of file)
Otherwise we can't really return a variable sized message with more than 65k
results. This was causing an integer overflow in `listchannels` (see #2504 for
details).
Signed-off-by: Christian Decker <decker.christian@gmail.com>
We were tarring up the build dir, not the destination dir! We did this
for 0.6.3 and nobody noticed :(
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@cdecker reports that this gives warnings on exit; and we can't suppress
them by setting ASAN_OPTIONS within the binary itself, unfortunately.
So for 0.7, disable it by default. I'll work through the errors for 0.7.1.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
And fix trivial typo in MAKING-RELEASES.md, and date retreival in
build-release.sh and repro-build.sh (real git tags start with v!)
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
I tried building zipfile on a fresh clone inside KVM, and got
1. Different times inside the zipfile, since zip seems to save *local* times.
2. A different zipfile order, since zip seems to use filesystem order.
Fix both of these. I don't know if LANG=C is necessary for git
ls-files, but it can't hurt.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
For the moment it's only Ubuntu 18.04.1.
Complete documentation is in the final commit; you can test this using
the prior commit and comparing with my intermediate files and results
at:
https://ozlabs.org/~rusty/clightning-repro
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Basically we tell it that every field ending in '_msat' is a struct
amount_msat, and 'satoshis' is an amount_sat. The exceptions are
channel_update's fee_base_msat which is a u32, and
final_incorrect_htlc_amount's incoming_htlc_amt which is also a
'struct amount_msat'.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
They're generally used pass-by-copy (unusual for C structs, but
convenient they're basically u64) and all possibly problematic
operations return WARN_UNUSED_RESULT bool to make you handle the
over/underflow cases.
The new #include in json.h means we bolt11.c sees the amount.h definition
of MSAT_PER_BTC, so delete its local version.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Christian and I both unwittingly used it in form:
*tal_arr_expand(&x) = tal(x, ...)
Since '=' isn't a sequence point, the compiler can (and does!) cache
the value of x, handing it to tal *after* tal_arr_expand() moves it
due to tal_resize().
The new version is somewhat less convenient to use, but doesn't have
this problem, since the assignment is always evaluated after the
resize.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
