RPC methods versioning for plugins

[darosior]

Rationale described here: #3354 (comment).

Summary: We may be going to rely on plugin-exposed RPC methods for crucial operations (such as Bitcoin backend), a versioning is useful to avoid nasty corner cases.

Open questions:

• should we allow to register multiple RPC methods with the same name, but different versions ?

[darosior]

Marking WIP as I shoudl also expose the version in list or get*info calls

[cdecker]

Marking WIP as I shoudl also expose the version in list or get*info calls

You can make a PR unmergeable if you use the "Create draft Pull request" button instead of the "Create Pull Request" button :-)

[darosior]

Changed my mind just after :/ I should make use of the "think before clicking the green button" button, too

[cdecker]

Changed my mind just after :/ I should make use of the "think before clicking the green button" button, too

No problem, I thought I might as well point it out for the future (also because I myself only found out about this recently) :-)

[darosior]

I think this is ready for review

[cdecker]

After mulling this over for a couple of days I am no longer convinced that
just adding a version number is what we want. Specifically there is no
connection between the version and the signature of the method/hook, which
kind of defeats the point. I've been playing with the idea of having a
meta-description of the signature (name + arguments + return structure) and
hash that to a version number, but it seems a bit too much at the moment.

It could also get rather confusing for plugin devs to distinguish which
version of the method/hook they need to implement. Maybe we should just
version in the method name (e.g. getblockbyheight1, getblockbyheight2,
...) and document them thoroughly in the docs instead of trying to paper over
the differences with code.

As long as we only add arguments or add more things to the returned results we
wouldn't need to change the method name either. This matches the current
semantics which allows plugins to return richer-than-expected results, and
suggests they should allow more arguments than strictly expected (everybody is
adding **kwargs at the end of the signature of pyln plugin methods and
hooks, right?).

Changing the name allows us to skip plugins that weren't updated with the
current signature in mind, and automatically fall back to the built-in
bitcoin-cli mechanism.

TL;DR: Maybe we should wait for the versioning to really become an issue, and when we have more information about how things might break.

I'm sorry I sent you down this rabbit hole by suggesting the versions in the
first place, but maybe simpler is also better?

[darosior]

everybody is adding **kwargs at the end of the signature of pyln plugin methods and
hooks, right?

Of course ! quickly adds a TODO line
More seriously (and a bit off-subject of this PR) maybe it's worth making YA round of updates for lightningd/plugins as we did with pyln replacement and tests ?

Changing the name allows us to skip plugins that weren't updated with the current signature in mind, and automatically fall back to the built-in bitcoin-cli mechanism.

That's what version numbers aimed to do, but I'm fine with names too.

Maybe we should wait for the versioning to really become an issue, and when we have more information about how things might break.

I agree, and I'm also in favor of adding less things as the diff for this is becoming really big on my side and it would just make review more complicated for a (somewhat?) premature optimisation.

I'm sorry I sent you down this rabbit hole by suggesting the versions in the
first place, but maybe simpler is also better?

Absolutely.

I'll PR the drive-by fixups commits and close this.

[ZmnSCPxj]

everybody is
adding **kwargs at the end of the signature of pyln plugin methods and
hooks, right?

/me looks guiltily around, then pretends nothing happened.