Additional tests for BIP 174 #702

achow101 commented at 2:04 AM on July 20, 2018: member

No description provided.

Additional tests for BIP 174 6107b01428

jb55 commented at 2:33 AM on July 20, 2018: none

I was meaning to mention this on the previous PRs. Is there any point to including the base64 encoding as well? The translation from hex is pretty straightforward. Just thinking it would cut down on the verbosity a bit.

achow101 commented at 6:05 PM on July 20, 2018: member

Is there any point to including the base64 encoding as well? The translation from hex is pretty straightforward.

The point was to effectively provide the two forms that a PSBT would be found in: as a binary file or as a Base64 string. It helps those who are writing tests to know exactly what to expect especially since there are (apparently) multiple ways of doing Base 64.

jb55 commented at 6:11 PM on July 20, 2018: none

there are (apparently) multiple ways of doing Base 64

https://en.wikipedia.org/wiki/Base64#Variants_summary_table

so it seems... seems reasonable to include it in that case, or alternatively an explicit mention of which variant is used.

achow101 commented at 7:43 AM on July 25, 2018: member

@luke-jr This is ready.

achow101 force-pushed on Aug 8, 2018

achow101 commented at 11:33 PM on August 8, 2018: member

I updated this with some parts that specify what signers should look out for and an algorithm for a simple signer. Also more tests for those.

in bip-0174.mediawiki:312 in fbd50980ee outdated

 304 | @@ -305,6 +305,47 @@ Signers do not need to sign for all possible input types. For example, a signer
 305 |  
 306 |  A single entity is likely to be both a Signer and an Updater as it can update a PSBT with necessary information prior to signing it.
 307 |  
 308 | +====Data Signers Check For====
 309 | +
 310 | +For a Signer to only produce valid signatures for what it expects to sign, it must check that the following conditions are true:
 311 | +
 312 | +* If a non-witness UTXO is provided, it's hash must match the hash specified in the prevout

sipa commented at 11:42 PM on August 8, 2018:

typo: "its".

achow101 commented at 12:17 AM on August 9, 2018:

Done

in bip-0174.mediawiki:334 in fbd50980ee outdated

 329 | +        if IsMine(key):
 330 | +            sign(non_witness_sighash(script_code, i, input))
 331 | +
 332 | +for input,i in enumerate(psbt.inputs):
 333 | +    if non_witness_utxo.exists:
 334 | +        assert(sha256d(non_witness_utxo) == prevhash)

sipa commented at 11:44 PM on August 8, 2018:

write prevhash as psbt.tx.input[i].prevout.hash?

achow101 commented at 12:17 AM on August 9, 2018:

Done

in bip-0174.mediawiki:336 in fbd50980ee outdated

 331 | +
 332 | +for input,i in enumerate(psbt.inputs):
 333 | +    if non_witness_utxo.exists:
 334 | +        assert(sha256d(non_witness_utxo) == prevhash)
 335 | +        if redeemScript.exists:
 336 | +            assert(non_witness_utxo.scriptPubKey == P2SH(redeemScript))

sipa commented at 11:45 PM on August 8, 2018:

Write as non_witness_utxo.vout[psbt.tx.input[i].prevout.n].scriptPubKey ?

achow101 commented at 12:17 AM on August 9, 2018:

Done

achow101 force-pushed on Aug 8, 2018

in bip-0174.mediawiki:313 in 85574c751c outdated

 304 | @@ -305,6 +305,53 @@ Signers do not need to sign for all possible input types. For example, a signer
 305 |  
 306 |  A single entity is likely to be both a Signer and an Updater as it can update a PSBT with necessary information prior to signing it.
 307 |  
 308 | +====Data Signers Check For====
 309 | +
 310 | +For a Signer to only produce valid signatures for what it expects to sign, it must check that the following conditions are true:
 311 | +
 312 | +* If a non-witness UTXO is provided, it's hash must match the hash specified in the prevout
 313 | +* If a witness UTXO is provided, a witness signature is being produced

sipa commented at 11:52 PM on August 8, 2018:

Perhaps write this as "If a witness UTXO is provided, no non-witness signature may be created"? The current language sounds like a signature must be created.

achow101 commented at 12:17 AM on August 9, 2018:

Done

in bip-0174.mediawiki:342 in 85574c751c outdated

 337 | +            sign_non_witness(redeemScript)
 338 | +        else:
 339 | +            sign_non_witness(non_witness_utxo.scriptPubKey)
 340 | +    else if witness_utxo.exists:
 341 | +        if redeemScript.exists:
 342 | +            assert(witness_utxos.scriptPubKey == P2SH(redeemScript))

sipa commented at 11:53 PM on August 8, 2018:

Typo: "witness_utxos".

achow101 commented at 12:17 AM on August 9, 2018:

Fixed

in bip-0174.mediawiki:347 in 85574c751c outdated

 342 | +            assert(witness_utxos.scriptPubKey == P2SH(redeemScript))
 343 | +            script = redeemScript
 344 | +        else:
 345 | +            script = witness_utxo.scriptPubKey
 346 | +        if IsP2WPKH(script):
 347 | +            sign_witness(P2PKH(script))

sipa commented at 11:53 PM on August 8, 2018:

You can't really convert a script to P2PKH. What about P2PKH(script[2:])?

achow101 commented at 12:18 AM on August 9, 2018:

Done

Specify what signers should check for and describe a simple signer

Test cases for what signers should check for

131fc2cebb

achow101 force-pushed on Aug 9, 2018

luke-jr merged this on Aug 9, 2018

luke-jr closed this on Aug 9, 2018