cleanup: do not use output outcome to return decryption shares #6673
Conversation
joschisan
force-pushed
the
lnv2_cleanup_outcome
branch
4 times, most recently
from
ded6d29 to
a6ca6d1
Compare
| @@ -583,6 +565,21 @@ impl ServerModule for Lightning { | |||
| Ok(module.await_preimage(db, params.0, params.1).await) | |||
| } | |||
| }, | |||
| api_endpoint! { | |||
| DECRYPTION_KEY_SHARE_ENDPOINT, | |||
| ApiVersion::new(0, 0), | |||
There was a problem hiding this comment.
These needs to point at the new, minor-increamented API version.
There was a problem hiding this comment.
Oh. Since it's on lnv2, we assume that nothing was released/deployed yet? OK.
There was a problem hiding this comment.
Yes, that would be the assumption. Just need to check with @elsirion on that...?
There was a problem hiding this comment.
As discussed on the dev call today: we could also change the major consensus version or the module kind
| @@ -881,12 +881,6 @@ pub trait ServerModule: Debug + Sized { | |||
| /// | |||
| /// Depending on the module this might contain data needed by the client to | |||
| /// access funds or give an estimate of when funds will be available. | |||
| /// | |||
There was a problem hiding this comment.
We should probably still give the correct guidance on the meaning of None, even if just to document the current state for our own purposes.
|
LGTM as well, would like the comment @dpc mentioned to be preserved |
|
@joschisan Comments + rebase. |
joschisan
force-pushed
the
lnv2_cleanup_outcome
branch
2 times, most recently
from
e30994f to
76cd3cb
Compare
| /// | ||
| /// In other words: after module has processed a given output it **MUST | ||
| /// NOT** return `None` for it, as it will lead to the panic. | ||
| /// Since this has become deprecated you may return None even if the output |
There was a problem hiding this comment.
I don't understand what "not used inside the module" means.
joschisan
force-pushed
the
lnv2_cleanup_outcome
branch
from
76cd3cb to
846d2a5
Compare
joschisan
force-pushed
the
lnv2_cleanup_outcome
branch
from
846d2a5 to
a772d8c
Compare
There was a problem hiding this comment.
Seems good to me. I'd like to prioritize #6465 after merging so when v0.6 is released we don't accidentally break lnv2
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
No description provided.