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

Guilt/sql plugin #10

Open
wants to merge 36 commits into
base: master
Choose a base branch
from
Open

Guilt/sql plugin #10

wants to merge 36 commits into from

Conversation

rustyrussell
Copy link
Owner

Test for ElementsProject#5679 which has run out of CI minutes?

rustyrussell and others added 29 commits January 30, 2023 16:30
Fixes d9fed06:

```
common/bolt11.c:868:31: error: format specifies type 'size_t' (aka 'unsigned long') but the argument has type 'u64' (aka 'unsigned long long') [-Werror,-Wformat]
                                           bech32_charset[type], field_len);
                                                                 ^~~~~~~~~
```

Signed-off-by: Rusty Russell <[email protected]>
This will allow gossipd to store and persist gossip for channels rather
than deleting them entirely when the channels are pruned from the
network.
Though BOLT 7 says a channel may be pruned when one side becomes inactive
and fails to refresh their channel_update, in practice, the
channel_announcement can be difficult to recover if deleted entirely.
Here the channel_announcement is tagged as zombie such that gossip_store
consumers may safely ignore it, but it may be retained should the channel
come back online in the future. Node_announcements and channel_updates may
also be retained in such a fashion until the channel is ready to be
resurrected.

Changelog-Fixed: Pruned channels are more reliably restored.
We removed the command for v22.11.

Also, we removed the `refund_for` offer parameter, so remove its description
from the manpage.

Signed-off-by: Rusty Russell <[email protected]>
…able.

We only ever use this table for output and input transactions: indeed, my node
doesn't have any annotation types 0.

Signed-off-by: Rusty Russell <[email protected]>
We didn't actually populate them properly, and the real annotations
are on inputs and outputs.

Signed-off-by: Rusty Russell <[email protected]>
Changelog-EXPERIMENTAL: JSON-RPC: `listtransactions` `channel` and `type` field removed at top level.
It's a core concept in the spec which isn't directly exposed.

Signed-off-by: Rusty Russell <[email protected]>
Changelog-Added: JSON-RPC: `listchannels` added a `direction` field (0 or 1) as per gossip specification.
`hash` is a tighter requirement than simply `hex`.

Signed-off-by: Rusty Russell <[email protected]>
We actually had a partid allowed (in the oneOf clauses), but didn''t
document it.

Signed-off-by: Rusty Russell <[email protected]>
It's actually two separate u16 fields, so actually treat it as
such!

Cleans up zombie handling code a bit too.

Signed-off-by: Rusty Russell <[email protected]>
This is useful when you're writing routines to scan it.

Signed-off-by: Rusty Russell <[email protected]>
connectd is the only one who uses these routines now.  The
rest can be linked into a plugin.

Signed-off-by: Rusty Russell <[email protected]>
We only handle top-level objects with an array of objects:
make sure it is one before we call the routines.

Signed-off-by: Rusty Russell <[email protected]>
I don't like it, but we do expose some times like this :(

Signed-off-by: Rusty Russell <[email protected]>
If we have a specific type (not just "hex") the length is implied.

Signed-off-by: Rusty Russell <[email protected]>
We have "secret" and "hash" types which are often more appropriate.

Signed-off-by: Rusty Russell <[email protected]>
This is designed to allow you to perform complex server-side
queries.

Signed-off-by: Rusty Russell <[email protected]>
Rather than two arrays "columns" (for names) and "fieldtypes" (for
types), use a struct.  This makes additions easier for successive
patches.

Also pull process_json_obj() out of the loop in list_done().

Signed-off-by: Rusty Russell <[email protected]>
This requires us to rename "index" fields, rename fields if we have a
sub-object, and create sub-tables if we have an array, and handle the
fact that some listX commands don't contain array X (listsendpays
contains "payments").

Signed-off-by: Rusty Russell <[email protected]>
Painfully created by hand from the source.

Signed-off-by: Rusty Russell <[email protected]>
…sip_store.

It's quite a lot of code, but these are the most expensive commands we
do so it's worth it.

Signed-off-by: Rusty Russell <[email protected]>
`"deprecated": true` is obsolete; we don't document them anyway.

Where it would have otherwise changed the GRPC wrappers, I actually put the
version number in.

We allow "listchannels" to have "satoshis" since we have some tests
that run in deprecated-api mode.

Signed-off-by: Rusty Russell <[email protected]>
For now, we ignore every deprecated field, but put in the logic so
that future deprecations will work as expected.

Signed-off-by: Rusty Russell <[email protected]>
We now add tables to the strmap as we allocate them, since we don't
want to call "finish_td" when we're merely invoked for the
documentation, and don't need a database.

Signed-off-by: Rusty Russell <[email protected]>
In particular, we generate the schema part from the plugin itself.

Signed-off-by: Rusty Russell <[email protected]>
Changelog-Added: Plugins: `sql` plugin command to perform server-side complex queries.
And document them!

Signed-off-by: Rusty Russell <[email protected]>
This *would* be a 1-line change (add it to Makefile) except that we
previously assumed a "list" prefix on commands.

These use the default refreshing, but they could be done better using
the time-range parameters.

Suggested-by: @niftynei
Signed-off-by: Rusty Russell <[email protected]>
Good for detection of what fields are present.

Signed-off-by: Rusty Russell <[email protected]>
Prompted by @ShahanaFarooqui's playing with examples and finding
common errors.

Signed-off-by: Rusty Russell <[email protected]>
Also, put the "added" lines in the request schemas for new commands:
this doesn't do anything (yet?) but it keeps `make schema-added-check` happy.

Signed-off-by: Rusty Russell <[email protected]>
rustyrussell pushed a commit that referenced this pull request Feb 14, 2023
This will fix a crash that I caused on armv7
and by looking inside the coredump with gdb
(by adding an assert on n that must be
different from null) I get the following stacktrace

```
(gdb) bt
\#0  0x00000000 in ?? ()
\#1  0x0043a038 in send_backtrace (why=0xbe9e3600 "FATAL SIGNAL 11") at common/daemon.c:36
\#2  0x0043a0ec in crashdump (sig=11) at common/daemon.c:46
\#3  <signal handler called>
\#4  0x00406d04 in node_announcement (map=0x938ecc, nann_off=495146) at common/gossmap.c:586
\#5  0x00406fec in map_catchup (map=0x938ecc, num_rejected=0xbe9e3a40) at common/gossmap.c:643
\#6  0x004073a4 in load_gossip_store (map=0x938ecc, num_rejected=0xbe9e3a40) at common/gossmap.c:697
\#7  0x00408244 in gossmap_load (ctx=0x0, filename=0x4e16b8 "gossip_store", num_channel_updates_rejected=0xbe9e3a40) at common/gossmap.c:976
\#8  0x0041a548 in init (p=0x93831c, buf=0x9399d4 "\n\n{\"jsonrpc\":\"2.0\",\"id\":\"cln:init#25\",\"method\":\"init\",\"params\":{\"options\":{},\"configuration\":{\"lightning-dir\":\"/home/vincent/.lightning/testnet\",\"rpc-file\":\"lightning-rpc\",\"startup\":true,\"network\":\"te"..., config=0x939cdc) at plugins/topology.c:622
\#9  0x0041e5d0 in handle_init (cmd=0x938934, buf=0x9399d4 "\n\n{\"jsonrpc\":\"2.0\",\"id\":\"cln:init#25\",\"method\":\"init\",\"params\":{\"options\":{},\"configuration\":{\"lightning-dir\":\"/home/vincent/.lightning/testnet\",\"rpc-file\":\"lightning-rpc\",\"startup\":true,\"network\":\"te"..., params=0x939c8c)
    at plugins/libplugin.c:1208
\#10 0x0041fc04 in ld_command_handle (plugin=0x93831c, toks=0x939bec) at plugins/libplugin.c:1572
\#11 0x00420050 in ld_read_json_one (plugin=0x93831c) at plugins/libplugin.c:1667
\#12 0x004201bc in ld_read_json (conn=0x9391c4, plugin=0x93831c) at plugins/libplugin.c:1687
\#13 0x004cb82c in next_plan (conn=0x9391c4, plan=0x9391d8) at ccan/ccan/io/io.c:59
\#14 0x004cc67c in do_plan (conn=0x9391c4, plan=0x9391d8, idle_on_epipe=false) at ccan/ccan/io/io.c:407
\ElementsProject#15 0x004cc6dc in io_ready (conn=0x9391c4, pollflags=1) at ccan/ccan/io/io.c:417
\ElementsProject#16 0x004cf8cc in io_loop (timers=0x9383c4, expired=0xbe9e3ce4) at ccan/ccan/io/poll.c:453
\ElementsProject#17 0x00420af4 in plugin_main (argv=0xbe9e3eb4, init=0x41a46c <init>, restartability=PLUGIN_STATIC, init_rpc=true, features=0x0, commands=0x6167e8 <commands>, num_commands=4, notif_subs=0x0, num_notif_subs=0, hook_subs=0x0, num_hook_subs=0, notif_topics=0x0, num_notif_topics=0) at plugins/libplugin.c:1891
\ElementsProject#18 0x0041a6f8 in main (argc=1, argv=0xbe9e3eb4) at plugins/topology.c:679
```

I do not know if this is a solution because I do not know
when I can parse a node announcement for a node that
it is not longer in the gossip map.

So, I hope this is just usefult for @rustyrussell

Changelog-Fixed: fixes `FATAL SIGNAL 11` on gossmap node announcement parsing.

Signed-off-by: Vincenzo Palazzo <[email protected]>
rustyrussell pushed a commit that referenced this pull request Apr 5, 2023
pubkey_from_hexstr() was failing, which we didn't notice because we
weren't checking the return value. The problem was that we were passing
it a strlen that was half the actual length.

Relevant error:

  [libsecp256k1] illegal argument: !secp256k1_fe_is_zero(&ge->x)

  ==417723== ERROR: libFuzzer: deadly signal
    #7 0x7f5deaacc7fb in abort
    #8 0x51b0b0 in secp256k1_default_illegal_callback_fn secp256k1.c
    #9 0x51bd8e in secp256k1_ec_pubkey_serialize
    #10 0x4e235b in pubkey_to_der bitcoin/pubkey.c:29:7
    #11 0x4e2941 in pubkey_cmp bitcoin/pubkey.c:89:2
    #12 0x4e333d in bitcoin_redeem_2of2 bitcoin/script.c:144:6
    #13 0x4f1396 in run tests/fuzz/fuzz-close_tx.c:78:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants