Skip to content
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

Merged
merged 16 commits into from
Sep 29, 2020

Conversation

nkohen
Copy link
Contributor

@nkohen nkohen commented Sep 9, 2020

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

@nkohen
Copy link
Contributor Author

nkohen commented Sep 9, 2020

I should note that the type numbers were picked randomly and are likely subject to change

@benthecarman
Copy link
Contributor

I should note that the type numbers were picked randomly and are likely subject to change

Maybe should put that in the doc

Messaging.md Outdated Show resolved Hide resolved
Messaging.md Outdated Show resolved Hide resolved
Messaging.md Outdated Show resolved Hide resolved
Messaging.md Outdated Show resolved Hide resolved
Copy link
Member

@Tibo-lg Tibo-lg left a 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.

Messaging.md Outdated Show resolved Hide resolved
Messaging.md Outdated Show resolved Hide resolved
Messaging.md Outdated Show resolved Hide resolved
Messaging.md Outdated Show resolved Hide resolved
Messaging.md Outdated Show resolved Hide resolved
Messaging.md Show resolved Hide resolved
Messaging.md Outdated Show resolved Hide resolved
Messaging.md Show resolved Hide resolved
@Tibo-lg Tibo-lg mentioned this pull request Sep 10, 2020
Messaging.md Outdated Show resolved Hide resolved
test/dlc_test.json Outdated Show resolved Hide resolved
test/dlc_test.json Outdated Show resolved Hide resolved
test/dlc_test.json Outdated Show resolved Hide resolved
test/dlc_test.json Outdated Show resolved Hide resolved
Messaging.md Outdated Show resolved Hide resolved
test/dlc_test.json Outdated Show resolved Hide resolved
@nkohen
Copy link
Contributor Author

nkohen commented Sep 25, 2020

The test vector does not pass the spec.
The spec says:

[oracle_info:oracle_info]
[point:funding_pubkey]

But the test vector seems to have the payout scriptPubKey instead after oracle_info.

I don't really know what you're referring to? That spec is for the dlc_offer message and I think you might be talking about the testInputs field which is very much not specified because it is only a test vector thing (where we have priv keys instead)?

@NicolasDorier
Copy link

@nkohen I am talking in the TLV offer message of the test vector.

The [ { .offer } ] which is specified in dlc_offer. The byte array I am parsing does not seem to use the same order of field than what is specified here.

@nkohen
Copy link
Contributor Author

nkohen commented Sep 25, 2020

@NicolasDorier Here is my parsing of the first offer in the file:

ByteVector(2 bytes, 0xa71a)
ByteVector(1 bytes, 0x00)
ByteVector(32 bytes, 0x0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206)
ByteVector(84 bytes, 0xfda7105064058b3477561188b9c89a0dbb8a843b797c4f4beb2a21dae4d3a1b54023b89e00000000000000004129cf1ed8463c29956c5edfe9e44de9f8da771f9f9b76f91897791a88bb2826000000000bebc200)
ByteVector(68 bytes, 0xfda71240ce266a66009b7e7b9f39c90ec77b6cc0cd7f58e087ed55c0b019545c44d38e2162fd055478041c159eb0b3b5ce3b9fbe28470522c6ac446873ed27f1c8dcf745)
ByteVector(33 bytes, 0x03efd58eb7b7c79fef1ff1edde8fb0b49c2358020097c016e05c17a129af01940f)
ByteVector(23 bytes, 0x160014c0bb4e0bfb0773843a7b4ff18a4a24e2312c3986)
ByteVector(8 bytes, 0x0000000005f5e100)
ByteVector(2 bytes, 0x0001)
ByteVector(100 bytes, 0xfda7146000520200000001b34e4345b6a07226f1aa91f2515096574fd6099eef1cd9885c8d069e5198cf760000000000000000000100c2eb0b000000001600146dd8f08240e4a3e25bfd3ebe43ae661b9cb3c02e0000000000000000ffffffff006c0000)
ByteVector(23 bytes, 0x160014b94dd1803e840d04b8f52332cff89ac6c42c4364)
ByteVector(8 bytes, 0x0000000000000005)
ByteVector(4 bytes, 0x00000064)
ByteVector(4 bytes, 0x000000c8)

You'll notice the 6th element is a pubkey as expected:

ByteVector(33 bytes, 0x03efd58eb7b7c79fef1ff1edde8fb0b49c2358020097c016e05c17a129af01940f)

@NicolasDorier
Copy link

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 :(

@NicolasDorier
Copy link

however, @nkohen the parsed version above might be useful to include in the BIP.


#### Version 0 `funding_input`

1. type: 42772 (`funding_input_v0`)

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.

Copy link
Contributor Author

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`]

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.

Copy link
Contributor Author

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

Messaging.md Outdated Show resolved Hide resolved
@NicolasDorier
Copy link

NicolasDorier commented Sep 28, 2020

Something wrong with vectors, I can't calculate the same temporary_contract_id as the Accept messages. I have the same byte array, tried all endianess possibilities.

The hash of the expected contract offer is different.

@NicolasDorier
Copy link

If you don't have anymore that's OK, but it would be useful to get the oracle private key.

@NicolasDorier
Copy link

Can you also provide the pre-images of the outcomes?

@nkohen nkohen changed the title Proposed v0 DLC TLV messages and added happy-path static test vectors Proposed v0 DLC TLV messages and Deterministic Fee Computation Sep 29, 2020
nkohen added a commit to nkohen/dlcspecs-1 that referenced this pull request Sep 29, 2020
@nkohen
Copy link
Contributor Author

nkohen commented Sep 29, 2020

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)

@nkohen nkohen merged commit 22b23eb into discreetlogcontracts:master Sep 29, 2020
nkohen added a commit to nkohen/dlcspecs-1 that referenced this pull request Sep 29, 2020
nkohen added a commit to nkohen/dlcspecs-1 that referenced this pull request Oct 5, 2020
nkohen added a commit to nkohen/dlcspecs-1 that referenced this pull request Oct 7, 2020
nkohen added a commit that referenced this pull request Oct 29, 2020
* 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants