hsmd: signer with splicing onchain checks for same splicing/dualfunding operations #6760
Conversation
|
if I read the code correctly, this doesn't follow the design in #6722 - it calls both the check and the lock in a single execution. the design intended to do both the check and the lock async and in different times in the negotiation flow. I haven't evaluated the behavior of the code, just wanted to raise the difference from the design on the initial review pass. |
|
this said, this approach is likely OK for an initial version, since the likelihood of needing to block should be relatively low. |
|
This looks good to me |
vincenzopalazzo
force-pushed
the
macros/remote-signer-with-splicing-onchain-checks
branch
2 times, most recently
from
c005367 to
9289037
Compare
While re-reading the code I found this async execution problem too, this can happen a low with VLS or any remote signer that has a different source. I really think that we should change it to a timer base solution, but the effort is not trivial because the openingd do not have any concepts of timers so we should implement it somehow. Before do this effort I would like to get some more review on the code. |
vincenzopalazzo
force-pushed
the
macros/remote-signer-with-splicing-onchain-checks
branch
from
9289037 to
1e19219
Compare
vincenzopalazzo
changed the title
vincenzopalazzo
force-pushed
the
macros/remote-signer-with-splicing-onchain-checks
branch
2 times, most recently
from
1e19219 to
2a573d2
Compare
vincenzopalazzo
force-pushed
the
macros/remote-signer-with-splicing-onchain-checks
branch
2 times, most recently
from
a636120 to
d3b69ca
Compare
In general, a validating signer may be under a different operational environment than the node, and therefore may have a different source of on-chain data. The signer may therefore temporarily disagree on whether a funding or splice transaction is locked (buried). We would like to ensure agreement between the signer and the node on how to progress a channel's state. The following message are added to provide a solution: - `check_outpoint(outpoint) -> bool` - check if the signer agrees that a funding candidate outpoint is buried - `lock_outpoint(outpoint)` - change the funding/splice state to locked Link: ElementsProject#6722 Suggested-by: @devrandom Co-Developed-by: Ken Sedgwick <[email protected]> Changelog-Added: hsmd protocol: Added hsmd_check_outpoint and hsmd_lock_outpoint Signed-off-by: Vincenzo Palazzo <[email protected]>
Tihis commit is implementing a 2-phase commit between the signer the node and the peer. The main reason for this is that everybody must agree on the lock, otherwise one of them will want N signatures (on the splice candidates), and another will produce only 1 signature. check_outpoint is the "prepare" for the signer, and lock_outpoint is the "commit". if check_outpoint returns true, lock_outpoint must not fail. Link: ElementsProject#6722 Suggested-by: @devrandom Co-Developed-by: Ken Sedgwick <[email protected]> Signed-off-by: Vincenzo Palazzo <[email protected]>
vincenzopalazzo
force-pushed
the
macros/remote-signer-with-splicing-onchain-checks
branch
from
d3b69ca to
1fc383e
Compare
|
ACK 1fc383e |
There was a problem hiding this comment.
Ack 1fc383e
It's simple, at least. Probably better to refactor this function into common/ but it's OK as is.
Yeah, I agree I tried to write it but there was something not similar between the two functions. However, we iterate a lot over it so maybe now it is possible :) I will take a look because keep just a function is also good for us |
vincenzopalazzo
deleted the
macros/remote-signer-with-splicing-onchain-checks
branch
This delta was meant to be part of ([ElementsProject#6760]), maybe lost in a rebase. Changelog-None
This delta was meant to be part of ([ElementsProject#6760]), maybe lost in a rebase. Changelog-None
This delta was meant to be part of ([#6760]), maybe lost in a rebase. Changelog-None
For more information about the problem see #6722
For the solution description see the commits
Fixes #6722