Uh oh!
There was an error while loading. Please reload this page.
smite-ir: implement BuildFundingCreated and SendFundingCreated operation - #120
Conversation
587a5d4 to
3c40a01Compare| Instruction { | ||
| operation: Operation::BuildFundingCreated, | ||
| inputs: vec![ | ||
| 10, 4, 5, 0, 1, 1, 1, 4, 12, 1, 1, 1, 1, 4, 12, 3, 5, 10, 1, 1, |
There was a problem hiding this comment.
Here, v4 is passed as the input with index 1 to BuildFundingCreated, which is funding_satoshis. This is also used by CreateFundingTransaction. Similar to checking funding_satoshis > push_msat, should it also be a mutator error if funding_satoshis does not match the funding tx output amount ?
This test should fail then:
test
diff --git a/smite-scenarios/src/executor.rs b/smite-scenarios/src/executor.rs
index 78cc5f5..82e4f17 100644
--- a/smite-scenarios/src/executor.rs+++ b/smite-scenarios/src/executor.rs@@ -1923,2 +1923,28 @@ mod tests {
+ #[test]+ fn execute_build_funding_created_funding_satoshis_tx_mismatch() {+ let mut instrs = build_and_send_funding_created_instructions();+ instrs[14] = Instruction {+ operation: Operation::LoadAmount(5_000_000),+ inputs: vec![],+ };+ instrs[15].inputs[1] = 14;+ let mut mock_cli = MockBitcoinCli {+ utxos: vec![sample_utxo()],+ change_spk: sample_change_spk(),+ ..Default::default()+ };+ let mut mock_conn = MockConnection::new();+ let ctx = &sample_context();+ let err = Executor::new(&mut mock_conn, &mut mock_cli, ctx)+ .execute(+ &Program {+ instructions: instrs,+ },+ std::time::Instant::now(),+ )+ .unwrap_err();+ assert!(matches!(err, ExecuteError::Commitment(_)));+ }+
// -- extract_field tests --There was a problem hiding this comment.
Ok, this was intentional. I am explicitly building the FundingCreated message with an arbitrary funding_satoshis value, which does not necessarily match the output value of the funding transaction. This allows us to test both invalid and valid cases. If this results in a CommitmentError, we explicitly ignore it in ir.rs, since it is a condition generated by the fuzzer and can be safely ignored.
We could have extracted funding_satoshis from the funding transaction instead, but that would largely restrict the operation to valid cases. In the future, I was thinking of splitting the funding transaction fields as well, so that we can provide a raw txid and vout separately and then mutate them while building the FundingCreated message.
There was a problem hiding this comment.
I've looked at the input list 3 times now and I'm pretty sure the indexes are way off. v10 has type FeeratePerKw which does not match the expected input type of FundingTransaction. v4 has type ShortChannelId which does not match the expected input type of Amount. etc.
There was a problem hiding this comment.
v4has typeShortChannelIdwhich does not match the expected input type ofAmount. etc.
Oh, you're right. I meant to comment on the input list at executor.rs:2007:
Instruction{operation:Operation::LoadAmount(u64::MAX),inputs:vec![],},Instruction{operation:Operation::BuildFundingCreated,inputs:vec![6,4,8,0,1,1,1,9,10,3,3,3,3,9,10,11,12,13,1,3,],},Instruction{operation:Operation::SendFundingCreated,inputs:vec![15],},There was a problem hiding this comment.
I've looked at the input list 3 times now and I'm pretty sure the indexes are way off.
v10has typeFeeratePerKwwhich does not match the expected input type ofFundingTransaction.v4has typeShortChannelIdwhich does not match the expected input type ofAmount. etc.
In 10497b1, operations were added in between without the indexes being updated accordingly. After the rebase, this wasn’t flagged, so it slipped through unnoticed. I’ll fix this and add assert_well_formed(&program); here so that any similar errors surface immediately
meant to comment on the input list at executor.rs:2007:
For @ekzyis, I was referring to executor.rs only, and I hope that answers your question
e426239 to
14b5ad1Compare| Instruction { | ||
| operation: Operation::BuildFundingCreated, | ||
| inputs: vec![ | ||
| 10, 4, 5, 0, 1, 1, 1, 4, 12, 1, 1, 1, 1, 4, 12, 3, 5, 10, 1, 1, |
There was a problem hiding this comment.
I've looked at the input list 3 times now and I'm pretty sure the indexes are way off. v10 has type FeeratePerKw which does not match the expected input type of FundingTransaction. v4 has type ShortChannelId which does not match the expected input type of Amount. etc.
| /// program context. Channel state starts empty. | ||
| pub fn new(conn: C, bitcoin_cli: B, context: ProgramContext) -> Self { | ||
| Self { | ||
| conn, | ||
| bitcoin_cli, | ||
| context, | ||
| channel_states: HashMap::new(), |
There was a problem hiding this comment.
We will eventually want a way to start with a non-empty channel state for some SnapshotSetups. But for now this is fine.
Uh oh!
There was an error while loading. Please reload this page.
Signed-off-by: Nishant Bansal <nishant.bansal.282003@gmail.com>
Signed-off-by: Nishant Bansal <nishant.bansal.282003@gmail.com>
Signed-off-by: Nishant Bansal <nishant.bansal.282003@gmail.com>
9874ae7 to
096c588CompareUh oh!
There was an error while loading. Please reload this page.
Depends-on: #117
This PR adds the
BuildFundingCreatedandSendFundingCreatedIR operations, which build and send afunding_createdmessage to the target peer.It also adds a new
channel_statesfield to theExecutorstruct. This field is used to store thechannel_statewhile building thefunding_createdmessage and is subsequently used to verify the receivedfunding_signedsignature.