Stellar: Proposal for Stellar Lumen support

[zulucrypto]

This is a proposal to add support for viewing and signing Stellar transactions

[prusnak]

Let's discuss here on creating the StellarSignTx message contents. Also, if Stellar and Ripple are similar (not sure if they are) we can create a message that would encapsulate message to be signed for both protocols.

[zulucrypto]

@prusnak Thank you for looking this over.

The only thing that needs to be signed for Stellar is a SHA256 hash of the bytes that make up the transaction.

However, one of the strengths of a hardware wallet is that you can verify what you're signing on the Trezor without having to trust the UI on the PC. To implement this, I'd need to accept a much larger blob of data, parse it on the Trezor, and display a human-readable version of the transactions on the screen.

A rough draft of the messages I'd need for Stellar:

SignStellarTx

message SignStellarTx {
	repeated uint32 address_n = 1;              // BIP-32 path to derive the key from master node
	required string networkPassphrase = 2;      // used to distinguish transactions on the testnet vs. public net
	optional string challenge_visual = 3;       // untrusted description of the transaction to show to the user, eg. "Pay Alice 3 XLM"
	required bytes blob = 4;                    // Blob of XDR data to be signed
}

The blob in this message would be raw bytes representing the transaction encoded with XDR[1] . I would write code on the Trezor that parses this and shows the user exactly what operations are in the transaction so they know what they're signing.

Once the user approved the operations, I'd generate the hash from the XDR blob and sign it.

SignedStellarTx

message SignedStellarTx {
	required bytes public_key = 2;  // public key for the private key used to sign data
	required bytes signature = 3;   // signature suitable for sending to the Stellar network
}

Pretty basic, nothing much necessary.

Regarding Ripple support, I believe Ripple can also use secp256k1 (Stellar only uses ed25519) so SignStellarTx would probably need another field for the curve. I'll do some more research on ripple transactions and see if the same transaction could work for both.

[1] https://www.stellar.org/developers/horizon/reference/xdr.html

[prusnak]

Looks nice. We need to replace blob with fields that will get encoded and hashed in the device and we are done. Also is challenge visual part of the data being hashed? If not, let's remove it from the message.

[zulucrypto]

This would be the message with a single field for the XDR:

message SignStellarTx {
	repeated uint32 address_n = 1;              // BIP-32 path to derive the key from master node
	required string networkPassphrase = 2;      // used to distinguish transactions on the testnet vs. public net
	required bytes xdr = 4;                    // Blob of XDR data to be signed
}

The fields that get hashed are the networkPassphrase and the xdr.

If you were thinking a separate field for each type of operation, that might be a bit tricker. Stellar has several operations with varying fields: https://www.stellar.org/developers/guides/concepts/list-of-operations.html

I believe they'd each require their own message type. For example:

message StellarOneOp {
    // Supported operation types
    enum Type { 
        CREATE_ACCOUNT = 1;
        PAYMENT = 2;
    }
    required Type type = 1;

    optional StellarOpCreateAccount createAccount = 2;
    optional StellarOpPayment       payment = 3;
}

message StellarOpCreateAccount {
    required bytes destination = 1;     // Public address of the account to create
    required bytes startingBalance = 2; // Amount of XLM to fund the new account with
}

message StellarOpPayment {
    required bytes destination = 1;     // Public address of the account to create
    required bytes asset = 2;           // Asset being transferred
    required bytes amount = 3;          // Amount of the asset being transferred
}

(Repeat for each available operation)

Then, SignStellarTx would look like:

message SignStellarTx {
    repeated uint32 address_n = 1;              // BIP-32 path to derive the key from master node
    required string networkPassphrase = 2;      // used to distinguish transactions on the testnet vs. public net
    repeated StellarOneOp operations = 3;       // Blob of XDR data to be signed
}

I'd originally avoided that because I thought it would be harder, but after figuring out the above I think it would make the parsing on the Trezor a lot easier if I just stuck with protobuf messages.

However, it would require a lot more message types. Let me know your thoughts!

[saleemrashid]

Can we rename this pull request to specify Stellar in the title?

[jhoenicke]

Can Stellar transactions have many operations? If yes, I'd propose to split the messages into multiple ones, like we do for the bitcoin clones. One StellarSignTx message that mentions how many operations and all the information needed to compute the first bytes of the xdr. Then the TREZOR asks back for each operation and a second message like StellarSignOpAck is used to send the operation's data. I assume the data is hashed in order, so that the TREZOR doesn't need to store the whole transaction but adds each operation immediately to the hash.

Another thing to decide: should the Trezor send back the xdr? For NEM and bitcoin we do this, we ethereum we don't do this. I think we shouldn't do it and just send back the signature. A decent wallet should know how to compute the xdr for itself.

[zulucrypto]

Can Stellar transactions have many operations?

Yes, although I think in 99% of cases there would only be one operation per transaction.

should the Trezor send back the xdr?

I agree that it shouldn't send back the full XDR and instead just the signature. I would like to avoid parsing XDR on the Trezor and leave that up to wallets which would already have an XDR parser built in.

Splitting it into a stream of multiple operations sounds good to me, although as an optimization I'd like to have StellarSignTx include the first operation in order to support the following workflow:

1. User is using a wallet to make a payment of 5 XLM to address G123. They click "sign transaction with Trezor"
2. Wallet sends transaction to Trezor with operation count of 1 and a StellarOpPayment as the first operation
3. Trezor displays a confirmation: "Send 5 XLM to G123?"
4. If user confirms, Trezor signs the transaction and returns the signature

In the case of multiple operations it would work as follows:

1. User is sending a transaction with two operations (payment of 5 XLM to G123 and payment of 10 XLM to G456)
2. Wallet sends transaction to Trezor with operation count of 2 and a StellarOpPayment as the first operation
3. Trezor displays a confirmation: "(1 of 2) Send 5 XLM to G123?" and two buttons: "Cancel" / "Next"
4. If the user presses "Next", Trezor requests the second operation and receives a StellarSignOpAck message with the next operation to sign
5. Trezor displays: "(2 of 2) Send 10 XLM to G456?" and two buttons: "Cancel" / "Finish"
6. If the user presses "Finish", Trezor signs the transaction

[zulucrypto]

@prusnak Stellar now has a standard for deriving accounts from mnemonic passphrases, so I believe I have everything I need to get started on this.

Since I'll be adding a fair number of messages (and it sounds like there are other people also working on adding coins) would it be possible to reserve message types 200-250 for Stellar? This could be as simple as adding a comment to messages.proto just as a hint to reduce conflicts and keep things organized.

[prusnak]

👍 cool!

Yes, let's reserve messagetypes 200-250 for Stellar/Ripple.

[zulucrypto]

@prusnak I've got this working with one of the operation types and since it's a fairly large PR I was wondering if you'd prefer reviewing it as early as possible or you'd rather I waited until I had all the features implemented?

So far, I have code for the following projects:

• trezor-common (adds new messages)
• trezor-mcu (adds signing support to firmware, the largest PR)
• python-trezor (adds stellar commands to trezorctl)

I'd also like to add support to the javascript libraries, but I haven't looked into that yet.

[prusnak]

I waited until I had all the features implemented?

What features are done and which are missing?

[zulucrypto]

@prusnak

Done:

• New layout functions
• All necessary protobuf messages
• Most of the protocol parsing code
• Transaction envelope parsing (operations are contained within a transaction envelope)
• Payment operation parsing
• trezorctl commands

To do:

• Parsing the rest of the operations (see: https://www.stellar.org/developers/guides/concepts/list-of-operations.html)
• Javascript support

The rest of the operations should be pretty straightforward, it's just a matter of parsing them and displaying confirmation messages.

[prusnak]

Wow! Great, let's review now. The 2 missing features can wait.

[prusnak]

@zulucrypto Is there anything I can help you with right now?

[zulucrypto]

@prusnak I think all I need is code review at this point. I've just submitted the PRs for trezor-common, python-trezor and trezor-mcu.

I'll be idling on gitter if that's easier than commenting in the PRs.

[prusnak]

Closing in favor of #63