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

Support 'EXTERNAL' for TransactionInput.ScriptType #38

Closed
RHavar opened this issue Oct 30, 2018 · 10 comments
Closed

Support 'EXTERNAL' for TransactionInput.ScriptType #38

RHavar opened this issue Oct 30, 2018 · 10 comments
Assignees
Labels
bitcoin Bitcoin related core Trezor Core firmware. Runs on Trezor Model T and T2B1.

Comments

@RHavar
Copy link

RHavar commented Oct 30, 2018

The firmware doesn't appear to support 'EXTERNAL' for TransactionInput.ScriptType, which is essential for coinjoins. I'm trying to build a simple bip79 front-end for trezor, but this appears to make it impossible.

Comment from issue: trezor/connect#247

prusnak wrote:

If you want to try implementing it, you should add another case when txi.script_type == InputScriptType.EXTERNAL here:
https://github.com/trezor/trezor-core/blob/b9926a9fffe8a593beb3adb79e5fc429233e41a3/src/apps/wallet/sign_tx/signing.py#L148

@stevenroose
Copy link

I'd also very much appreciate EXTERNAL. Currently my workaround is to just put SPENDWITNESS and then remove the signature again afterwards.

@prusnak
Copy link
Member

prusnak commented Nov 13, 2018

This is not planned at the moment. The issue will be solved via PSBT support which will be added in the future: https://github.com/trezor/trezor-core/issues/410

@prusnak prusnak closed this as completed Nov 13, 2018
@matejcik
Copy link
Contributor

matejcik commented Nov 14, 2018

Reopening as PSBT support solves a slightly different problem.

To get to the same page: EXTERNAL script type is supposed to mark an input to not be signed by Trezor. Instead, the user explicitly provides a scriptSig which is used when building the transaction.

For some usecases, a possible workaround is using SPENDWITNESS script type, allow Trezor to sign the input, and then replace the Trezor-generated signature in the result.

@matejcik matejcik reopened this Nov 14, 2018
@matejcik
Copy link
Contributor

For the record, this might end up a prerequisite for signing PSBTs, if Trezor doesn't parse the binary blobs itself. A partially signed transaction might already contain signatures from other sources.

This also means some, uh, challenges, on the UI side. How do we display a transaction where we're sending 1 BTC out of 3 BTC total?

@stevenroose
Copy link

Partially signing transaction. Do you want to sign off on spending 4.2 BTC from your wallet in the transaction totalling 42 BTC?
[y]
This transaction sends 22 BTC to address 123...
[enter]
This transaction sends 20 BTC to address 156...
[enter]
Confirm?
[y]

Or otherwise perhaps
Partially signing transaction that sends 42 BTC in total to the following addresses...
[enter]
This transaction sends 22 BTC to address 123...
[enter]
This transaction sends 20 BTC to address 156...
[enter]
Do you want to contribute 4.2 BTC from your wallet for this transaction?
[y]

@tsusanka
Copy link
Contributor

tsusanka commented Apr 5, 2019

I'm moving this to backlog, but we'll most likely include this into the next milestone.

@prusnak
Copy link
Member

prusnak commented Apr 7, 2019

As per https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2017-August/014843.html it is necessary to add an ownership proof to every EXTERNAL input.

@matejcik
Copy link
Contributor

matejcik commented Apr 8, 2019

It is still possible to sign EXTERNAL inputs without ownership proofs, given that they are treated as risky and confirmed explicitly by the user.

This (esp. the segwit part) also means that we would require prev_tx data for every EXTERNAL input, so that we can validate the ownership proof signature and validate the amount for segwit inputs. Not great - maybe the ownership proof could be extended to cover the amount too?

In any case this probably stops this feature until we have some sort of standard for the proof of ownership.

@prusnak prusnak transferred this issue from trezor/trezor-core Apr 16, 2019
@prusnak prusnak added this to the 2019-06 milestone Apr 16, 2019
@prusnak prusnak added architecture core Trezor Core firmware. Runs on Trezor Model T and T2B1. bitcoin Bitcoin related labels Apr 16, 2019
@tsusanka tsusanka modified the milestones: 2019-06, 2019-07 Apr 26, 2019
@ZdenekSL ZdenekSL modified the milestones: 2019-07 , backlog Jun 7, 2019
@ZdenekSL ZdenekSL modified the milestones: backlog, 2019-Q4 Jun 25, 2019
@prusnak prusnak removed the packages label Jun 26, 2019
@tsusanka tsusanka removed this from the 2019-Q4 milestone Sep 20, 2019
@tsusanka tsusanka added this to the backlog milestone Sep 20, 2019
@prusnak
Copy link
Member

prusnak commented Oct 10, 2019

Let's refactor the signing code first: #617

@tsusanka
Copy link
Contributor

tsusanka commented Jun 9, 2020

Implementing SLIP-19 is needed for this. Moved to #1052.

@tsusanka tsusanka closed this as completed Jun 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bitcoin Bitcoin related core Trezor Core firmware. Runs on Trezor Model T and T2B1.
Projects
None yet
Development

No branches or pull requests

6 participants