Decred support

[saleemrashid]

Support for normal Decred transactions.

I had hoped to use SigHashAllValue but, as @davecgh has confirmed, it is disabled at the moment. Therefore, we have to use STAGE_REQUEST_2_PREV_*.

However, a Decred signature hash is a hash over hash_type || hash_prefix || hash_witness. As hash_prefix is calculated as part of Phase 1, we do not have to assert that the transaction has not changed in either Phase 2 or Phase 3.

Furthermore, as the Decred prefix does not contain signatures, we can serialize the prefix in Phase 1 and skip Phase 2 entirely.

• Transaction signing
• Multisignature transaction signing
• Checking previous output script version (see @davecgh's comment)
• Transaction weight computation
• Signed messages

Requires trezor/trezor-common#66

/cc @peterzen

[davecgh]

It might make sense to fail to sign any transaction inputs for which the previous output script version is not 0.

I suggest this because in the future the plan is to have a consensus vote which will enable a new signature hash algorithm that is more efficient and only applies to v1+ scripts. Assuming that change ends up getting voted in, attempts to sign transaction inputs with v1 scripts would result in an invalid signature with the current code in the PR. Obviously this wouldn't result in anything dire since the transaction would just be rejected due to an invalid signature, however I think it probably provides a better user experience to report that only v0 scripts are supported and the user should look for an updated firmware versus just producing a transaction that ends up getting rejected with an invalid signature which would likely be confusing for the user.

EDIT: Modified to use the versions in the code per the comment below to avoid confusion.

[saleemrashid]

@davecgh By script version 1, you mean decred_script_version == 0, right?

[davecgh]

@saleemrashid: Right. I should've used the v0 and v1+ in my comment since that's how it is in the code as opposed to the human-readable 1st and 2nd versions.

[saleemrashid]

In Phase 1, we are serializing the prefix and computing hash_prefix before asking the user for confirmation. Then we skip directly to Phase 3 and serialize the witnesses which are signatures over hash_type || hash_prefix || hash_witness. Therefore, as far as I can tell, there shouldn't be any risk of the transaction changing during signing.

/cc @jhoenicke

[saleemrashid]

@davecgh So, I could add a decred_script_version to TxInputType as well. And if it tries to sign a TxInputType with decred_script_version > 0, it will fail saying that the script version is unsupported. There's no reason to check that decred_script_version corresponds to the previous output decred_script_version because, if it doesn't, the signatures should be invalid, right?

[prusnak]

I had hoped to use SigHashAllValue but, as @davecgh has confirmed

When will it be enabled? Can we wait until this is enabled on the network? It would make the user experience much better.

[davecgh]

@saleemrashid That sounds correct, assuming TxInputType is unique per transaction input as I believe it is. To clarify a bit more, the script version is defined on a transaction output, and thus a transaction input that spends said transaction output is responsible for generating signature scripts which conform to the semantics of that script language version. The script version of the previous output is indeed committed to by the prefix, so if you produce a signature which commits to a script version which does not match the previous output, it will fail signature verification.

@prusnak Unfortunately, it can't be enabled at the current time because of the strictness checks which are part of the consensus rules prevent it, even though the signature algorithm itself technically supports it. Changing it would be a hard fork, which in Decred, means there would need to be a consensus vote and accompanying DCP. Consequently, since it requires a consensus vote to change anyways, we might as well change the algorithm altogether to improve its efficiency.

Along these lines, I would actually like to hear from you guys working on Trezor what else you think would be useful in the signature algorithm to improve the Trezor user experience. For example, I know at a bare minimum the signature should also commit to the amount of the current input, but I suspect that it makes sense to commit to all input values (aka total overall transaction input amount) so the Trezor can safely use an untrusted total amount in calculations such as transaction fees without also needing to be able to verify all of the other signatures, which is, of course, only possible if all inputs are owned by the signer.

[saleemrashid]

@davecgh The most efficient method would probably be to commit to all_input_amounts || signature_script because we can cache the all_input_amounts rather than asking for each input again (as SigHashAllValue would require).

That being said, writing this has made me realise that I can make this PR more efficient (I still have some leftovers from SigHashAllValue that mean I ask for the inputs when I don't need to)

[davecgh]

@saleemrashid Thanks for the clarification. That is what I suspected. When I work up a DCP to define an updated signature hash algorithm, I'll make sure it includes a commitment to all input values. If there are any other changes that would allow the Trezor code to be more efficient and help improve its UX, please let me know so I can be sure to include them as well.

[saleemrashid]

I've removed the round-trip in Phase 3 which was previously necessary for SigHashAllValue. I will write some more tests for this now.

[davecgh]

For reference, I created an issue to discuss a new signature hash algorithm for the future. It doesn't directly affect this PR as it would be a while (several months) before any new algorithm would be put up for vote and voted in via the stakeholders, however, I'm putting a link here to provide everyone working on the trezor integration ample opportunity to request any changes.

decred/dcrd#950

[saleemrashid]

The script version of the previous output is indeed committed to by the prefix

@davecgh The prefix of the spending transaction? I don't think it is.

[davecgh]

@saleemrashid You are correct that I was mistaken there. I actually noticed that earlier myself when going through it for the purposes of 950 I linked above and, if you've reviewed that yet, you'll noticed I made sure it's included in there in the proposal for the new algorithm. In the current algo, it's committed to in the ouputs, but not for the input script. It isn't an issue though since only v0 scripts are supported currently and v1 scripts will use the new algorithm which is incompatible with the old one, so the signature would fail regardless.

[saleemrashid]

@davecgh Does the last commit address your concerns about the script version? It aborts signing if the input has a script version of v1+ or if it was told the input has a script version of v0 but the previous output has a script version of v1+.

If it does, then I think, after trezor/trezor-common#69, trezor/python-trezor#182 and trezor/trezor-crypto#125 have been merged, this is ready to be merged!

[davecgh]

@saleemrashid That looks correct to me. Thanks!

[saleemrashid]

Updated trezor-crypto submodule to master.

@prusnak This is ready for review now.

[bradneuman]

Any update on this? Looks like it was being considered for 1.6.1, but that came and went, and now there are submodule conflicts

[saleemrashid]

Rebased on master, requires trezor/trezor-crypto#145

[prusnak]

Thank you!