Skip to content

smite-ir: add CreateChannelConfig IR operation - #103

Closed
NishantBansal2003 wants to merge 1 commit into
lnfuzz:masterfrom
NishantBansal2003:create-chan-config-ir
Closed

smite-ir: add CreateChannelConfig IR operation#103
NishantBansal2003 wants to merge 1 commit into
lnfuzz:masterfrom
NishantBansal2003:create-chan-config-ir

Conversation

@NishantBansal2003

Copy link
Copy Markdown
Contributor

ref: #90 (comment)
Depends-on: #100

This commit introduces a new compute operation CreateChannelConfig, which takes the static channel and party inputs and constructs a new variable type ChannelConfig. This will later be used as an input while constructing the BuildFundingCreated message.

Some reasons why I went with CreateChannelConfig operation in funding flow:

  • This helps keep inputs cleaner for subsequent operations. For example, BuildFundingCreated can now directly take ChannelConfig instead of many raw inputs.
  • Another reason for not building funding_created directly from raw inputs is signature verification. Once we receive funding_signed, we need the same ChannelConfig to verify the received signature. Reconstructing it again from raw inputs could lead to false positives if the reconstructed config differs from the original. By reusing the previously created ChannelConfig, verification stays consistent and avoids such issues.

Some points worth mentioning are:

  • With this approach, mutations from mutators effectively shift from BuildFundingCreated to CreateChannelConfig. So I think we will still receive the same mutation coverage as before. After construction, ChannelConfig becomes immutable, which helps preserve the exact configuration for later signature verification. Because of that, I do not think there is any real tradeoff from the mutator side.
  • Its usage for subsequent normal channel operations is still an open discussion point, and I plan to open a separate issue to finalize that design.

This commit introduces a new compute operation, `CreateChannelConfig`,
which constructs a `ChannelConfig` variable from static channel and
party inputs. This `ChannelConfig` is then used by `BuildFundingCreated`.
The main motivation for introducing `CreateChannelConfig` is to keep
subsequent operations cleaner and avoid repeatedly passing many raw
inputs. It also ensures that the exact same configuration used to build
`funding_created` is later reused for verifying `funding_signed`, avoiding
inconsistencies or false positives from reconstructing the config again
during verification.
From the mutator perspective, mutation coverage is effectively preserved,
with mutations shifting from `BuildFundingCreated` to `CreateChannelConfig`.
Since `ChannelConfig` becomes immutable after construction, it also guarantees
stable inputs for later signature verification.
Signed-off-by: Nishant Bansal <nishant.bansal.282003@gmail.com>
@NishantBansal2003

Copy link
Copy Markdown
ContributorAuthor

Based on the discussion in #111, I think we should have the following IR operations for the funding flow:

Since we're not going with the variable types ChannelConfig and CommitmentState in the normal channel operations, and instead storing them implicitly in the executor as immutable state, there’s no need to define them as variables for the funding flow either.

  • BuildFundingCreated: Builds the funding_created message using the raw inputs.
  • RecvFundingSigned: As previously planned, we should verify the peer's signature. But, we can't do that now because ChannelConfig and CommitmentState are not represented in the IR, and reconstructing them could produce a different state and cause verification to fail. I think it's reasonable to skip this verification in the funding flow for now and perform it later in the normal channel operations. Though later, once we have channel_states stored in the executor, we can construct the ChannelConfig and CommitmentState in BuildFundingCreated, store them in channel_state, and then use them in RecvFundingSigned to verify the peer’s signature.
  • BuildChannelReady
  • RecvChannelReady

If this design makes sense, let me know whether I should update this PR with BuildFundingCreated (since a lot of the implementation will be the same) or close it and open a new PR instead?

@morehouse

Copy link
Copy Markdown
Collaborator
* `RecvFundingSigned`: As previously planned, we should verify the peer's signature. But, we can't do that now because `ChannelConfig` and `CommitmentState` are not represented in the IR, and reconstructing them could produce a different state and cause verification to fail. I think it's reasonable to skip this verification in the funding flow for now and perform it later in the normal channel operations. Though later, once we have `channel_states` stored in the `executor`, we can construct the `ChannelConfig` and `CommitmentState` in `BuildFundingCreated`, store them in `channel_state`, and then use them in `RecvFundingSigned` to verify the peer’s signature.

I think we should still be able to verify the signature (and store it for future force closes). Basically when BuildFundingCreated is executed, the executor needs to create the initial entry for the channel in its map. That entry will hold ChannelConfig, HolderIdentity, initial CommitmentState, etc. When the funding_signed message is received later, the executor looks up the channel state for the given channel_id and verifies the signature.

If this design makes sense, let me know whether I should update this PR with BuildFundingCreated (since a lot of the implementation will be the same) or close it and open a new PR instead?

Let's open a new PR.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@NishantBansal2003@morehouse