-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposed v0 DLC TLV messages and Deterministic Fee Computation #81
Proposed v0 DLC TLV messages and Deterministic Fee Computation #81
Conversation
I should note that the type numbers were picked randomly and are likely subject to change |
2d22efd
to
585dc7b
Compare
Maybe should put that in the doc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I put some comments about things that could likely be optimized, but we can iterate over this in the future.
… signature specification
4920c2a
to
5bb66db
Compare
I don't really know what you're referring to? That spec is for the |
@nkohen I am talking in the TLV offer message of the test vector. The |
@NicolasDorier Here is my parsing of the first offer in the file:
You'll notice the 6th element is a pubkey as expected:
|
WTF so sorry @nkohen that was actually my implementation that mixed things up... I inverted in my test assertion actual and expected, so I got confused. Sorry for wasting your time :( |
however, @nkohen the parsed version above might be useful to include in the BIP. |
|
||
#### Version 0 `funding_input` | ||
|
||
1. type: 42772 (`funding_input_v0`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is quite confusing, in contract_info_v0
we have a single TLV record for many outcomes. Here we have one TLV record for each input.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because contract information follows a uniform type (all outcomes fall under the same version, and if they don't a future version will have separate TLV records) whereas different inputs are independent (e.g. taproot input could be funding_input_v1
and not have prev_tx
at the same time as another input is v0
)
|
||
1. type: 42772 (`funding_input_v0`) | ||
2. data: | ||
* [`u16`:`prevtx_len`] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in theory not needed, as a parser knows how big is a transaction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this is to allow clients which don't have transaction parser to make external calls to one after having parsed the transaction. Regardless of why I have made this record as close to this as possible: https://github.com/niftynei/lightning-rfc/pull/1/files#diff-3369c5aa1774fef2ff1e246979f223eaR169
Something wrong with vectors, I can't calculate the same The hash of the expected contract offer is different. |
If you don't have anymore that's OK, but it would be useful to get the oracle private key. |
Can you also provide the pre-images of the outcomes? |
Going to merge spec changes only (test vectors now in #100) as these seem to be acked by basically everyone (and of course, we will iterate on the spec in future PRs) |
* Added test vectors deleted from #81 * Fixed dlc_test redeemscript and added new dlc message parsing tests * Fixed chain_hash endianness * Added Schnorr signature point computation test vectors * Restructured contract info json * Restructured signature TLVs dlc_message_test.json * UInt16 prefixed scripts and added hash pre-images to test vectors * Added Nicolas' tests for NFC normalization and hashing * Added dlc fee computation tests * Added clarification to redeemscript in funding_input_v0 wrt fee computation * Added basic tx building tests * Updated test vectors to use 107 witness bytes for P2WPKH * Updated test vectors to include non-p2wpkh inputs as well as dummy scripts to be used in fee tests for less modular APIs * Added maxWitnessLen to inputs explicitly * Fixed order and serialization of signatures * Fixed backward stack funding signatures
Begins work on implementing #47, #61, #62, #72, #76
Implements #12
EDIT: Used to implement static test vectors but these have been moved to another PR