Fix simple to_local revoked output claim and rebase #184 #199
Conversation
| assert_eq!(revoked_local_txn[0].input[0].previous_output.txid, chan_5.3.txid()); | ||
| assert_eq!(revoked_local_txn[0].output.len(), 2); // Only HTLC and output back to 0 are present | ||
| assert_eq!(revoked_local_txn[1].input.len(), 1); | ||
| assert_eq!(revoked_local_txn[1].input[0].previous_output.txid, revoked_local_txn[0].txid()); |
There was a problem hiding this comment.
Check that it's a HTLC-Timeout with assert_eq!(..witness.len(), 5) and (..witness[5].len(), 133) ?
| let header = BlockHeader { version: 0x20000000, prev_blockhash: Default::default(), merkle_root: Default::default(), time: 42, bits: 42, nonce: 42 }; | ||
| nodes[1].chain_monitor.block_connected_with_filtering(&Block { header, txdata: vec![revoked_local_txn[0].clone()] }, 1); | ||
| let node_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap(); | ||
| assert_eq!(node_txn.len(), 3); // nodes[1] will broadcast justice tx twice, and its own local state once |
There was a problem hiding this comment.
side-thought: Aren't duplicate (or more) tx generation due to block re-scan not gonna be a bottleneck for watchtowers ?
There was a problem hiding this comment.
Hmm, I suppose it might, though hopefully watchtowers aren't broadcasting that many transactions to begin with as people would have to be misbehaving for that.
| @@ -3510,6 +3509,9 @@ mod tests { | |||
|
|
|||
| // Get the will-be-revoked local txn from node[0] | |||
| let revoked_local_txn = nodes[0].node.channel_state.lock().unwrap().by_id.get(&chan_1.2).unwrap().last_local_commitment_txn.clone(); | |||
| assert_eq!(revoked_local_txn.len(), 2); // commitment tx + 1 HTLC-Timeout tx | |||
| assert_eq!(revoked_local_txn[0].input.len(), 1); | |||
| assert_eq!(revoked_local_txn[0].input[0].previous_output.txid, chan_1.3.txid()); | |||
There was a problem hiding this comment.
same as above, maybe ensure that the witnessScript is an offered htlc one?
|
utACK, fix + test of revoked to_local output is something less to be done in #137 ! |
TheBlueMatt
force-pushed
the
2018-09-184-fixed-monitor
branch
from
a2aabf6 to
af29adc
Compare
|
Ok, fixed the comments, squashed and macro'd the tx.verify() map stuff (adding one more such test for local txn spending each other). |
|
Good, thanks for the macro, will use it for next tests! |
In review of #184 the tests looked funky and it turned out we were incorrectly claiming revoked to_local outputs. FIrst commit fixes it, then follows a rebase of #184 with the fixes required separated out so its easier to review the differences (I'll squash them before merge).