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

Add script_pubkey field to TxInput message #1857

Merged
merged 1 commit into from Oct 18, 2021

Conversation

andrewkozlik
Copy link
Contributor

Signing Taproot UTXOs involves computing the new BIP341 hash, which hashes all previous scriptPubKeys, not just the one we are signing. If we own the input, then we can (and should) derive its scriptPubKey inside Trezor. If it's an external input, then we need the scriptPubKey to be supplied. We can already do that with the present API by requesting the corresponding PrevOutput, i.e. just the one previous output, not the entire previous transaction. An alternative is to change the API by adding a script_pubkey field to the TxInput message, which will have to be filled in for external inputs only. We have decided to go with the latter option. For the record, the reasons considered are summarized below.

Reasons to keep the API as is:

  • The extra PrevOutput message will be needed only for external inputs (once for signature hashing and once for verification of externality). Apparently the only disadvantage is that in CoinJoin it will mean 2*(N-1) extra messages during signing, where N is the number of participants, which seems acceptable.
  • The factor of size. By adding script_pubkey to TxInput we increase the maximum size of the message by another 520 bytes. TxInput is already the biggest message type we have with a theoretical maximum size of 7767 bytes and a practical maximum of ~4000 bytes.
  • The PrevOutput solution seems nicer from a defensive coding point of view. we wouldn't need to check yet another optional field, we're guaranteed that an attacker isn't slipping in a value that gets accidentally used when it shouldn't, etc.

Reasons to add the script_pubkey field to the TxInput message:

  • Fewer messages exchanged between host PC and Trezor.
  • In the future this will make the API simpler for apps that use Trezor, because they won't have to implement PrevOutput message handling if they only sign Taproot inputs.
  • In TxInput we already have fields like witness and ownership_proof which have very similar rules, e.g. ownership_proof may be present only for external inputs. The main difference between the addition of these fields and the question of adding script_pubkey is that there exists an alternative way to get script_pubkey via PrevOutput.
  • TxInput already has the field amount, which also wouldn't have to be there and could be obtained via PrevOutput. The main difference between amount and script_pubkey is that amount is mandatory for every input whereas script_pubkey only for external inputs.
  • Slightly simpler implementation of Taproot signing in legacy, where requesting the PrevInput means adding a new state to the state machine.

@andrewkozlik andrewkozlik force-pushed the andrewkozlik/scriptpubkey-field branch from 669fc1e to ba6de3b Compare October 18, 2021 14:03
@andrewkozlik andrewkozlik marked this pull request as ready for review October 18, 2021 14:50
@andrewkozlik andrewkozlik removed the request for review from prusnak October 18, 2021 14:50
@andrewkozlik andrewkozlik force-pushed the andrewkozlik/scriptpubkey-field branch from ba6de3b to 9ddb42f Compare October 18, 2021 14:52
@andrewkozlik andrewkozlik merged commit 13cbb59 into master Oct 18, 2021
@andrewkozlik andrewkozlik deleted the andrewkozlik/scriptpubkey-field branch October 18, 2021 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants