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

Commando: "Somewhere, somehow, someone's going to pay." #5370

Merged
merged 18 commits into from
Jul 16, 2022

Conversation

rustyrussell
Copy link
Contributor

@rustyrussell rustyrussell commented Jul 3, 2022

This is, honestly, the greatest plugin ever written.

It allows use of the already-established Lightning network protocol to control a node, and in addition supports authorization cookies (called "runes") which are easy to craft and powerful. Runes can restrict what commands can be run, what nodes can run them, what parameters can be given, limit what time the rune is valid for, and ratelimit how often the rune can be used. You can decode them into human readable form, and add new restrictions to them.

This is a reimplementation (and streamlining) of the commando.py plugin. It is always on, but doesn't do anything until you create your first rune.

It is fully documented, too. With examples!

Credit for naming: @cdecker

@rustyrussell rustyrussell added the Optech Make Me Famous! Look! Look! Look! COOL NEW FEATURE! label Jul 3, 2022
@rustyrussell rustyrussell added this to the v0.12 milestone Jul 3, 2022
@rustyrussell rustyrussell requested a review from jb55 July 3, 2022 12:37
@rustyrussell rustyrussell force-pushed the guilt/commando branch 2 times, most recently from 88cc109 to 256ac56 Compare July 4, 2022 00:15
@rustyrussell
Copy link
Contributor Author

Rebased on #5735:

@jb55
Copy link
Collaborator

jb55 commented Jul 11, 2022

this is amazing! testing now

@rustyrussell rustyrussell force-pushed the guilt/commando branch 2 times, most recently from b21c8e2 to aa0c750 Compare July 12, 2022 12:36
@rustyrussell
Copy link
Contributor Author

Merged typo fix by @jb55 and rebased on master (again!) to fix some flakes...

@rustyrussell rustyrussell force-pushed the guilt/commando branch 4 times, most recently from f8cbb8f to a2f38ac Compare July 15, 2022 03:01
@rustyrussell
Copy link
Contributor Author

Rebase on master, thanks to some prereqs being merged from @adi2011 !

@rustyrussell
Copy link
Contributor Author

Needs rebase now that makesharesecret is in with different API. See #5430

@rustyrussell
Copy link
Contributor Author

Rebased on #5430

@jb55
Copy link
Collaborator

jb55 commented Jul 15, 2022

I turned off the plugin and ran this branch and all my apps still work. amazing.

@jb55
Copy link
Collaborator

jb55 commented Jul 15, 2022

and it's fast 🤯

looks like guilt got confused and made me the author on 74785ce ?

@rustyrussell
Copy link
Contributor Author

and it's fast 🤯

looks like guilt got confused and made me the author on 74785ce ?

Ah, I squashed your fix in there. I'll fix, especially since I have to rebase to fix conflicts anyway...

The linker discards whole files in an archive if it doesn't need them,
so saves a bit of space (and time).  Also allows us to add more niche
things to CCAN (e.g. runes support!) without bloating all the binaries.

We also had many places which depended on $(CCAN_FILES), but that was
already a dependent of $(ALL_PROGRAMS) and $(ALL_TEST_PROGRAMS).

Before:

```
$ size lightningd/lightning*d
   text	   data	    bss	    dec	    hex	filename
2247683	   8696	  39008	2295387	 23065b	lightningd/lightning_channeld
2086607	   7432	  38880	2132919	 208bb7	lightningd/lightning_closingd
2227916	   8056	  39200	2275172	 22b764	lightningd/lightning_connectd
3369236	 119288	  39240	3527764	 35d454	lightningd/lightningd
2183551	   8352	  38880	2230783	 2209ff	lightningd/lightning_dualopend
2196389	   8024	  39136	2243549	 223bdd	lightningd/lightning_gossipd
2086216	   7488	  39264	2132968	 208be8	lightningd/lightning_hsmd
2134396	   8136	  39424	2181956	 214b44	lightningd/lightning_onchaind
2133391	   8352	  38880	2180623	 21460f	lightningd/lightning_openingd
1512168	   2136	  34384	1548688	 17a190	lightningd/lightning_websocketd
```

After:
```
text	   data	    bss	    dec	    hex	filename
2192065	   8488	  38912	2239465	 222be9	lightningd/lightning_channeld
2030957	   7224	  38816	2076997	 1fb145	lightningd/lightning_closingd
2179571	   7968	  39104	2226643	 21f9d3	lightningd/lightning_connectd
3354296	 119288	  39208	3512792	 3599d8	lightningd/lightningd
2127933	   8144	  38816	2174893	 212fad	lightningd/lightning_dualopend
2141699	   7856	  39072	2188627	 216553	lightningd/lightning_gossipd
2024482	   7288	   5240	2037010	 1f1512	lightningd/lightning_hsmd
2072074	   7920	   5400	2085394	 1fd212	lightningd/lightning_onchaind
2077773	   8144	  38816	2124733	 206bbd	lightningd/lightning_openingd
1408958	   1752	    344	1411054	 1587ee	lightningd/lightning_websocketd
```

Signed-off-by: Rusty Russell <[email protected]>
This is useful when have have a jsmntok_t.

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

commando wants to see the whole reply object, and also not to assume params is
an object.

Signed-off-by: Rusty Russell <[email protected]>
Plugins are supposed to store their data in the datastore, and commando does so:
let's make it easier for them by providing convenience APIs.

Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell
Copy link
Contributor Author

Rebased on master, fixed @jb55 fix which made him author of the plugin :)

rustyrussell and others added 13 commits July 16, 2022 22:48
Signed-off-by: Rusty Russell <[email protected]
Changelog-Added: Plugins: `commando` a new builtin plugin to send/recv peer commands over the lightning network, using runes.
This is needed for invoice, which can be asked to commit to giant descriptions
(though that's antisocial!).

Signed-off-by: Rusty Russell <[email protected]>
Some JSON error include "data", and we should reflect that.

Signed-off-by: Rusty Russell <[email protected]>
We support the old commando.py plugin, which stores a random secret,
as well as a more modern approach which uses makesecret.

Signed-off-by: Rusty Russell <[email protected]>
Can both mint new runes, and add one or more restrictions to existing ones.

Signed-off-by: Rusty Russell <[email protected]>
This means we can leave commando on by default, without an explicit config flag.

Signed-off-by: Rusty Russell <[email protected]>
Awkward to filter, but they're really practical for many commands.

Signed-off-by: Rusty Russell <[email protected]>
I'm assuming that nobody wants a rate slower than 1 per minute; we can
introduce 'drate' if we want a per-day kind of limit.

Signed-off-by: Rusty Russell <[email protected]>
This is a bit weird since it lives in the offers plugin, but it works
well.  This should make runes much more approachable for people!

Signed-off-by: Rusty Russell <[email protected]>
Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 2e13b72

The following test failure looks unrelated, and also looks like untracked? cc @niftynei

=================================== FAILURES ===================================
__________________________ test_v2_open_sigs_restart ___________________________
[gw0] linux -- Python 3.7.13 /opt/hostedtoolcache/Python/3.7.13/x64/bin/python3
node_factory = <pyln.testing.utils.NodeFactory object at 0x7f5e8b394d10>
bitcoind = <pyln.testing.utils.BitcoinD object at 0x7f5e8ac14490>
@unittest.skipIf(TEST_NETWORK != 'regtest', 'elementsd doesnt yet support PSBT features we need')
@pytest.mark.developer("uses dev-disconnect")
@pytest.mark.openchannel('v2')
deftest_v2_open_sigs_restart(node_factory, bitcoind):
        disconnects_1 = ['-WIRE_TX_SIGNATURES']
        disconnects_2 = ['+WIRE_TX_SIGNATURES']
        l1, l2 = node_factory.get_nodes(2,
                                        opts=[{'disconnect': disconnects_1,
'may_reconnect': True},
                                              {'disconnect': disconnects_2,
'may_reconnect': True}])
        l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
        amount = 2**24
        chan_amount = 100000
        bitcoind.rpc.sendtoaddress(l1.rpc.newaddr()['bech32'], amount / 10**8 + 0.01)
        bitcoind.generate_block(1)
# Wait for it to arrive.
        wait_for(lambda: len(l1.rpc.listfunds()['outputs']) > 0)
# Fund the channel, should appear to finish ok even though the
# peer fails
with pytest.raises(RpcError):
            l1.rpc.fundchannel(l2.info['id'], chan_amount)
        chan_id = first_channel_id(l1, l2)
        log = l1.daemon.is_in_log('{} psbt'.format(chan_id))
assert log
        psbt = re.search("psbt (.*)", log).group(1)
        l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
        l1.daemon.wait_for_log('Peer has reconnected, state DUALOPEND_OPEN_INIT')
with pytest.raises(RpcError):
>           l1.rpc.openchannel_signed(chan_id, psbt)
E           Failed: DID NOT RAISE <class 'pyln.client.lightning.RpcError'>

@rustyrussell rustyrussell merged commit 58ae885 into ElementsProject:master Jul 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Optech Make Me Famous! Look! Look! Look! COOL NEW FEATURE!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants