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

Update ContractInfoTLVs in test vectors to use pre-images #125

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

nkohen
Copy link
Contributor

@nkohen nkohen commented Dec 18, 2020

They were still using hashes before this PR instead of bigsize prefixed UTF-8 encoded strings with NFC normalization.

I had to regenerate dlc_message_test.json from scratch because I didn't have access to the pre-images of the hashes there.

Note for the future: dlc_message_test.json was really valuable to me in finding what the diff was between the old test vectors and the new ones on bitcoin-s :)

@@ -47,9 +47,9 @@
"payoutAddress" : "bcrt1qx2rdxteeum3suhqvxgzwvyljtjjv5nd4g24vuy"
}
},
"offer" : "a71a0006226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910ffda71050582e987a29cec9b8fd24a03935b415c9a2afa88059f31c3fcd1e013b8a698316000000000bebc200c6f1e739d6321d0728b8fa72b9005af83d30479aa68d76be8245ce5f06138fe60000000000000000fda71240937ce52b968e2bf92253fda2932ffdcf41b4a4a4840b1ae6dfb086fc0c11e90d5966a509a8244c6fde25ed5eb677b2fc79e2889371bff864790ff6af8055966d03b221c54dbb518d6a954845059b4a8e98e422109cc3437e705eb54538361967e00016001480b036d07462465cfc592c5cb896d226fdb01fe10000000005f5e1000001fda7146000520200000001b34e4345b6a07226f1aa91f2515096574fd6099eef1cd9885c8d069e5198cf760000000000000000000100c2eb0b00000000160014c1e616cb35cdb08332d9608f3d9d2dbf4ec34be80000000000000000ffffffff006b000000160014384e043abf63c513347c9e6b9aeb2f8f7ffd5da2000000000000000500000064000000c8",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you take this opportunity to rename testInputs to inputs if that is not controversial?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also nit, but IIUC, the structure of contract info will be:

outcome: String
localPayout: u64

So it might be better to rename things in the test input so that outcome is the same value that should go in the contractInfo struct (what is currently preImage), and rename what is currently called outcome to hashedOutcome or something like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
v0.1
  
In progress
Development

Successfully merging this pull request may close these issues.

None yet

2 participants