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

Runes migration from commando to lightning #6403

Merged
merged 9 commits into from
Jul 21, 2023
Merged

Runes migration from commando to lightning #6403

merged 9 commits into from
Jul 21, 2023

Conversation

ShahanaFarooqui
Copy link
Collaborator

@ShahanaFarooqui ShahanaFarooqui commented Jul 15, 2023

This moves the runes functionality of CLN from commando into the core (it does not yet redirect the commando plugin to use the core, but that's next!). This allows runes to be use by far more infrastructure, such as the coming clnrest plugin!

Changelog-Added: JSON-RPC: checkrune: check rune validity for authorization; createrune to create/modify rune; listrunes to list existing runes; blacklistrune to revoke permission of rune

Changelog-Deprecated: JSON-RPC: commando-rune, commando-listrunes and commando-blacklist.

@rustyrussell
Copy link
Contributor

Changelog-Changed: JSON-RPC: commando-rune to createrune, commando-listrunes to listrunes, commando-blacklist to blacklistrune : rune functionality moved into core from commando plugin.

Changelog-Added: JSON-RPC: checkrune: check rune validity for authorization

These need to be in commit messages: we shouldn't count on getting them from GH.

Also, it's Changelog-Added: for the new ones, and Changelog-Deprecated: for the old ones!

@rustyrussell
Copy link
Contributor

rustyrussell commented Jul 15, 2023

Oh, and don't merged inside PRs! Always rebase, if you need to...

(And you do actually need to, since master changed a little: deprecated_apis becomes ld->deprecated_apis...

@rustyrussell rustyrussell added this to the v23.08 milestone Jul 15, 2023
@rustyrussell rustyrussell added the Optech Make Me Famous! Look! Look! Look! COOL NEW FEATURE! label Jul 15, 2023
@MiWCryptAnalytics
Copy link

Any chance we could consider renaming the blacklistrune function? This is a user facing name, and I think we have an opportunity to be creative -- Its a rune, an ancient stone of great power and magical authorization, could it be: Destroyed, Shattered, Disintegrated, Obliterated, Pulverized, Smashed, Splintered, Unbound, Defaced, Demagicked, Disenchanted, Depowered ...

@rustyrussell
Copy link
Contributor

"uncastrune"?

@rustyrussell
Copy link
Contributor

Also needs rebase to fix db ops which have changed...

@rustyrussell
Copy link
Contributor

"uncastrune"?

"curserune". But I'll let Shahana decide :) (Note: we can have aliases to commands fairly trivially, by replicating the struct json_command)

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Most code stolen from commando, but uses db directly not datastore.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This looks suspiciously like `commando-rune`!

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This extracts the core checking functionality for a rune, so they can
easily be used more widely than just commando.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell
Copy link
Contributor

Weird! postgres doesn't like "end" as a column name, so @ShahanaFarooqui suggested "start_index" and "end_index".

@rustyrussell
Copy link
Contributor

Ack 1b0a88c

ShahanaFarooqui and others added 3 commits July 21, 2023 10:55
…ed commando ones.

Rune functionality moved into core from commando plugin.

Changelog-Added: JSON-RPC: `checkrune`: check rune validity for authorization; `createrune` to create/modify rune; `listrunes` to list existing runes; `blacklistrune` to revoke permission of rune

Changelog-Deprecated: JSON-RPC: `commando-rune`, `commando-listrunes` and `commando-blacklist`.

No-schema-diff-check
This is neater anyway, but we still need to tell memleak code about the htable.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
@rustyrussell rustyrussell merged commit 0ceee93 into ElementsProject:master Jul 21, 2023
@tonyaldon
Copy link
Contributor

If createrune, listrunes and blacklistrune do the same thing as respectively commando-rune, commando-listrunes and commando-blacklist, why don't alias them in order not to break apps that depend on those names?

As mentioned above, it just requires replicating 3 struct json_command.

I think this has 2 benefits:

Am I missing something about aliasing? What do you think?

@ShahanaFarooqui
Copy link
Collaborator Author

ShahanaFarooqui commented Jul 24, 2023

Any chance we could consider renaming the blacklistrune function? This is a user facing name, and I think we have an opportunity to be creative -- Its a rune, an ancient stone of great power and magical authorization, could it be: Destroyed, Shattered, Disintegrated, Obliterated, Pulverized, Smashed, Splintered, Unbound, Defaced, Demagicked, Disenchanted, Depowered ...

Thank you for the suggestion.

  • Added invokerune command alias for createrune
  • Added destroyrune alias for command blacklistrune.
    with commit 48e5f40.

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.

4 participants