smite-ir: add CreateFundingTransaction IR operation

[NishantBansal2003]

This commit adds the CreateFundingTransaction IR operation. For this, a new VariableType, FundingTransaction, has been added. This is required because we need the funding transaction after receiving the funding signed message so that we can broadcast the transaction and exchange channel_ready messages

[morehouse]

Code looks good.

What's your vision for fitting CreateFundingTransaction into the funding flow?

I'm guessing we'll implement a BroadcastTx operation next that takes a FundingTransaction as an input? Does it make sense to have separate operations, or should we just combine into a single CreateAndBroadcastFundingTx? Or would it make sense to have a generic Transaction variable type instead of the more specific FundingTransaction?

[morehouse]

Nit: should we call it BuildFundingTransaction for consistency with similar operations?

[NishantBansal2003]

Hmm, actually I did not use Build* because we defined Build operations as Build: construct a BOLT message from inputs, so I thought using it here might be confusing. Since this is more of a compute operation, I thought of using Create* instead. But I'm fine with using Build* or maybe some other name if you'd prefer that

[morehouse]

Create* is fine with me as well for this category of operations. I don't think it matters much, as long as we establish a consistent pattern as we extend the IR.

[NishantBansal2003]

What's your vision for fitting CreateFundingTransaction into the funding flow?

So I was thinking we would have the following IR operations for the funding flow:

• CreateFundingTransaction: Creates the funding tx. This is required for funding_created (to reference the funding outpoint) and later for broadcasting after funding_signed is exchanged.
• BroadcastFundingTransaction: Broadcasts the funding tx after funding_signed.
• CreateChannelConfig: Along with a ChannelConfig variable type, this would be constructed once after the open_channel <-> accept_channel flow and later used to reference the channel. This can later be extended for normal channel operations with something like LoadChannelConfigFromContext, or by introducing Load*FromContext inputs (as mentioned in the sample ProgramContext in issue IR Design and Implementation Plan #5 ) that help construct CreateChannelConfig.
• CreateInitialCommitmentState/CreateCommitmentState: Along with CommitmentState variable type, this would construct the initial commitment state. I still need to think more about the design here, but this could later be extended for normal channel operations via something like an AdvanceCommitmentState/NextCommitmentState IR operation. The CommitmentState could also later be loaded through something like LoadInitialCommitmentStateFromContext/LoadCommitmentStateFromContext after initializing normal channel scenario, or can later be constructed in the same way from inputs in ProgramContext, as done in CreateChannelConfig.
• BuildFundingCreated: Builds the funding_created message using ChannelConfig, CommitmentState.
• RecvFundingSigned: Uses ChannelConfig, CommitmentState to verify the received signature, and the output_type would be ChannelId , so there is no need for a separate ExtractFundingSigned operation (We could have ExtractFundingSigned operation purely to extract the ChannelId for consistency, WDYT here?)
• BuildChannelReady
• RecvChannelReady

I'm guessing we'll implement a BroadcastTx operation next that takes a FundingTransaction as an input? Does it make sense to have separate operations, or should we just combine into a single CreateAndBroadcastFundingTx? Or would it make sense to have a generic Transaction variable type instead of the more specific FundingTransaction?

I mean, we can combine them, and initially I was thinking the same thing, but that would be less aligned with the LN flow, since there the broadcast happens only after funding_signed. Still, I’m fine with it if you want to keep it that way.

I created FundingTransaction because I need both the transaction itself and the vout to be used in funding_created, otherwise, we could simply use Transaction. But I'm not fully sure here -- if in the future we always create the tx output ourselves, then we could maybe just hardcode the vout to 0. WDYT?
With that, we could have something like either of these:

• CreateAndBroadcastFundingTx: creates the transaction, broadcasts it, and returns the transaction as the output_type (Or we could simply merge this into the CreateChannelConfig case mentioned above)
• CreateTx and BroadcastTx separately

[morehouse]

* `CreateFundingTransaction`: Creates the funding tx. This is required for `funding_created` (to reference the funding outpoint) and later for broadcasting after `funding_signed` is exchanged.

* `BroadcastFundingTransaction`: Broadcasts the funding tx after `funding_signed`.

* `CreateChannelConfig`: Along with a `ChannelConfig` variable type, this would be constructed once after the `open_channel <-> accept_channel` flow and later used to reference the channel. This can later be extended for normal channel operations with something like `LoadChannelConfigFromContext`, or by introducing `Load*FromContext` inputs (as mentioned in the sample `ProgramContext` in issue [IR Design and Implementation Plan #5](https://github.com/morehouse/smite/issues/5) ) that help construct `CreateChannelConfig`.

* `CreateInitialCommitmentState/CreateCommitmentState`: Along with `CommitmentState` variable type, this would construct the initial commitment state. I still need to think more about the design here, but this could later be extended for normal channel operations via something like an `AdvanceCommitmentState/NextCommitmentState` IR operation. The `CommitmentState` could also later be loaded through something like `LoadInitialCommitmentStateFromContext/LoadCommitmentStateFromContext` after initializing normal channel scenario, or can later be constructed in the same way from inputs in `ProgramContext`, as done in `CreateChannelConfig`.

* `BuildFundingCreated`: Builds the `funding_created` message using `ChannelConfig`, `CommitmentState`.

This all sounds good. My only concern is that we might miss out on some bugs if we always generate funding_created and later HTLC messages from ChannelConfig and CommitmentState. It would be good if we can somehow allow mutators to directly modify the message fields before we send them.

* `RecvFundingSigned`: Uses `ChannelConfig`, `CommitmentState` to verify the received signature, and the `output_type` would be `ChannelId` , so there is no need for a separate `ExtractFundingSigned` operation (We could have `ExtractFundingSigned` operation purely to extract the `ChannelId` for consistency, WDYT here?)

We actually might need to extract the signature field from funding_signed as well for future channel-operations fuzzing or force close fuzzing.

* `BuildChannelReady`

* `RecvChannelReady`

I'm guessing we'll implement a BroadcastTx operation next that takes a FundingTransaction as an input? Does it make sense to have separate operations, or should we just combine into a single CreateAndBroadcastFundingTx? Or would it make sense to have a generic Transaction variable type instead of the more specific FundingTransaction?

I mean, we can combine them, and initially I was thinking the same thing, but that would be less aligned with the LN flow, since there the broadcast happens only after funding_signed. Still, I’m fine with it if you want to keep it that way.

No, I think your approach is better since it allows us to generate unexpected flows like broadcasting and mining out of order (before receiving funding_signed, or even before sending funding_created), in case there are state machine bugs that can be exposed that way.

I created FundingTransaction because I need both the transaction itself and the vout to be used in funding_created, otherwise, we could simply use Transaction. But I'm not fully sure here -- if in the future we always create the tx output ourselves, then we could maybe just hardcode the vout to 0. WDYT? With that, we could have something like either of these:

* `CreateAndBroadcastFundingTx`: creates the transaction, broadcasts it, and returns the `transaction` as the `output_type` (Or we could simply merge this into the `CreateChannelConfig` case mentioned above)

* `CreateTx` and `BroadcastTx` separately

I like the idea of a BroadcastTx that can broadcast any Transaction, but your explanation makes sense for why we would want a separate FundingTransaction type. I suggest that we start with FundingTransaction and wait until later (once we actually have other transaction types) to figure out how we might generalize to just Transaction. We could still call it BroadcastTx if we want; it would just take a FundingTransaction input for now.

[NishantBansal2003]

This all sounds good. My only concern is that we might miss out on some bugs if we always generate funding_created and later HTLC messages from ChannelConfig and CommitmentState.It would be good if we can somehow allow mutators to directly modify the message fields before we send them.

Yes, I was thinking whether we can support this case in mutator? I think with this approach we can exercise a lot of different cases (including valid ones), which we currently are not covering. For example, right now we will always send the correct funding outpoint, but with this approach we could also exercise invalid outpoints. That would also allow us to use ChannelConfig and CommitmentState even for funding_created and HTLC-related operations.

We actually might need to extract the signature field from funding_signed as well for future channel-operations fuzzing or force close fuzzing.

Yep, I think for the funding flow we can proceed with the MVP approach, and in the future, if we need access to signatures, we can always add dedicated Extract* operations later

Another thing I wanted to discuss is the generator. I think for the funding flow, we might actually need to rename OpenChannelGenerator to something like FundingFlowGenerator. Otherwise, if we create separate generators for funding_created and channel_ready, we will not be able to exercise the full flow until we have more interesting mutators like SpliceMutator or GeneratorInsertionMutator.

PS: I was thinking of opening an HTLC design/implementation plan issue so we can discuss extending the funding flow into normal channel operations, mainly around CommitmentState and mutator design.

[morehouse]

LGTM

[morehouse]

This all sounds good. My only concern is that we might miss out on some bugs if we always generate funding_created and later HTLC messages from ChannelConfig and CommitmentState.It would be good if we can somehow allow mutators to directly modify the message fields before we send them.

Yes, I was thinking whether we can support this case in mutator? I think with this approach we can exercise a lot of different cases (including valid ones), which we currently are not covering. For example, right now we will always send the correct funding outpoint, but with this approach we could also exercise invalid outpoints. That would also allow us to use ChannelConfig and CommitmentState even for funding_created and HTLC-related operations.

It's tricky -- raw-bytes mutation on the message itself isn't possible since we don't build the messages until runtime. So we need to enable mutators to instead change the individual input variables to the Build* operations. If the input variables are ChannelConfig and CommitmentState, then mutators need a way to create entirely new ChannelConfigs and CommitmentStates to swap with the correct inputs.

Yep, I think for the funding flow we can proceed with the MVP approach, and in the future, if we need access to signatures, we can always add dedicated Extract* operations later

SGTM

Another thing I wanted to discuss is the generator. I think for the funding flow, we might actually need to rename OpenChannelGenerator to something like FundingFlowGenerator. Otherwise, if we create separate generators for funding_created and channel_ready, we will not be able to exercise the full flow until we have more interesting mutators like SpliceMutator or GeneratorInsertionMutator.

It might be worth having both -- OpenChannelGenerator would only do the partial open_channel -> accept_channel flow, while FundingFlowGenerator would do the full flow.

I haven't thought too carefully about it, but it may also make sense to have individual generators for each funding flow message, and then have FundingFlowGenerator compose the individual generators into the full flow. Then (once we have splice and generator-insertion) we can easily construct programs that send messages in unexpected orders as well as the expected funding-flow sequence.

PS: I was thinking of opening an HTLC design/implementation plan issue so we can discuss extending the funding flow into normal channel operations, mainly around CommitmentState and mutator design.

Sounds good.