Implement Stellar support

[tsusanka]

Stellar has an Account Viewer, which is basically a very thin web wallet, which supports Ledger. It's a great place to implement Trezor Connect to add Trezor support.

@szymonlesisz - @zulucrypto would like to implement this. Do you think you could provide him with some high-level instructions what has to be done to make this happen?

[zulucrypto]

@szymonlesisz - Hello, any chance you've had some time to write up some instructions or should I wait until the next version of the firmware is officially released to implement this?

[szymonlesisz]

hi @zulucrypto,
I'm already working on it. I just need to fix some bugs and release it to public.
I hope it will be during this week.

[zulucrypto]

That's great @szymonlesisz , thanks!

[zulucrypto]

@szymonlesisz - It looks like many of the messages are changing to use a string for the addresses instead of raw bytes: trezor/trezor-common#135

I'm guessing this will change your implementation a bit, could you let me know if that's the case?

[szymonlesisz]

Sure, if anything change i will let you know.
I can tell you right now that fields for StellarSignTx method definitely will be changed. Right now those fields are 1:1 with protobuf, but for code consistency inside trezor-connect project field names should be camelCase instead of snake_case.
Im still looking for common model of Stellar Transaction object (that object you are putting into .stellarSignTransaction method)
I was checking js-stellar-sdk https://github.com/stellar/js-stellar-sdk/blob/master/src/transaction_call_builder.js and it looks like they have 2 different types (account transactions + ledger transactions)
Do you think you can help me out with this a little bit and suggest how this transaction should look like (from the stellar wallet point of view) to avoid unnecessary field converting/mapping?

[s-a-y]

@szymonlesisz it's not 2 different types, it's the same type, just a filter when fetching transaction history.
Real deal is here https://github.com/stellar/js-stellar-base/blob/master/src/transaction_builder.js#L100
Returned value is the one that is being signed.

[szymonlesisz]

I've just wrote a proposition of types for Stellar Transaction object.
You can find it here
It's based on types from Stellar-sdk

Few fields are named differently comparing to Trezor protobuf messages that's why needs to be mapped inside trezor-connect. I've wrote a comments for each field with corresponding protobuf field value.
Still there are some fields missing in stellar-sdk but required in protobuf and i don't know from where exactly (in stellar-sdk) those values should coming from.
@zulucrypto, since you defined those messages in protobuf, maybe you can help me out with that?

I've merged "LedgerRecord" and "TransactionRecord" into one "Transaction" type but i have few unknown fields in there:

Transaction:
sequence: number; // is it a field from "LedgerRecord": "sequence" ? or from "TransactionRecord" "source_account_sequence" ? > Proto: "sequence_number"

timebounds_start: number, // IDK this field from stellar API? Is it from "TransactionRecord" "created_at"?

timebounds_end: number, // IDK this field from stellar API ?

memo: string; // Stellar API has only "memo_text" type? Because in protobuf i can declare "memo_type", "memo_id", "memo_id" and "memo_text"

max_fee: number; // from "TransactionRecord"?

When we solve this we can discuss unknown/missing fields in Operations.

Please let me know what you think about it

[s-a-y]

@szymonlesisz have you seen my comment above? You're looking at the wrong class TransactionRecord and LedgerRecord are not relevant, they are used to fetch transactions from history.
Building new transactions in stellar-sdk happens in js-stellar-base library
Here's transaction object
https://github.com/stellar/js-stellar-base/blob/master/src/transaction.js

It has sequence, timebounds, memo (defined here), fee

[szymonlesisz]

@s-a-y, sorry my bad, somehow i've missed that.
I've updated my code to correspond with Transaction from js-stellar-base.
You can see it here

so final call for signing transaction will look like:

TrezorConnect.stellarSignTransaction({
path: Array | string; // bip44 "m/44'/148'/0'" format or hardended array [ 44 | 0x80000000, 148 | 0x80000000, 0 | 0x80000000 ]
ledgerVersion: number;
networkPassphrase: string;
transaction: Transaction; // types declared above
})

Now i'm left with only one issue:

• SetOptionsOperation: i didn't find signer_type field in Stellar API but it's present in protobuf messages.
• both ManageOfferOperation and PassiveOfferOperation have "price" field, but in protobuf messages are 2 fields: "price_n" and "price_d", both in "uint32" type. I'm not sure which one should be mapped from incoming parameters or how to use them?
• BumpSequenceOperation is missing in Stellar API but it's present in protobuf messages
• InflationOperation is missing in protobuf messages but present in Stellar API

any thoughts?

[tsusanka]

• BumpSequenceOperation is missing in Stellar API but it's present in protobuf messages
• InflationOperation is missing in protobuf messages but present in Stellar API

Those two were discussed here: trezor/trezor-core#202 (comment)

[szymonlesisz]

Ok, i've find out that price should be an object with "d" and "n" fields.

@param {number} opts.price.n - If opts.price is an object: the price numerator
@param {number} opts.price.d - If opts.price is an object: the price denominator

Only 1 issue left :)

[zulucrypto]

@szymonlesisz "signer type" will be one of these constants:

• 0 - an account is being added/removed as a signer
• 1 - a pre-authorized transaction hash is being added/removed as a signer
• 2 - a hash is being added/removed as a signer

Some background is available here: https://www.stellar.org/developers/guides/concepts/multi-sig.html#additional-signing-keys

[s-a-y]

Hey, any update?
I'm dying here.

Anything I can help with?

[zulucrypto]

Hi @szymonlesisz - is there anything you're waiting on me for or anything I can help with?

[szymonlesisz]

Hi @zulucrypto,

It's already implemented in version 5:

https://github.com/trezor/connect/blob/develop/docs/methods/stellarGetAddress.md
https://github.com/trezor/connect/blob/develop/docs/methods/stellarSignTransaction.md

you can give it a try here:

https://trezor.github.io/connect-explorer/#/stellar-getaddress

or

https://trezor.github.io/connect-explorer/#/stellar-signtx

[s-a-y]

[zulucrypto]

@szymonlesisz - Great news! One minor issue that I noticed is that the protocol_version field was removed from the protobuf messages recently. So, you can remove ledgerVersion and the associated docs.

I'll give it a closer look as soon as I can, thanks for getting it together!

[zulucrypto]

Hi @szymonlesisz - I just tried it out with the latest firmware from master and "get address" works great!

I did get an error when trying to submit a transaction:

StellarSignTx#protocol_version is not a field: undefined

I'm guessing this is because protocol_version was removed from the protobuf messages recently, so hopefully it's an easy fix.

The addresses were also changed to be strings instead of the raw bytes, so you may need to update the code to directly pass them instead of using base64.

As an example:

Before

source: "5d55642466b185b843152e9e219151dbc5892027ec40101a517bed5ca030c2e0",

After

source: "GDRXE2BQUC3AZNPVFSCEZ76NJ3WWL25FYFK6RGZGIEKWE4SOOHSUJUJ6",

Thanks again for working on this!

[szymonlesisz]

fixed in 5.0.30

[zulucrypto]

Looks like it's working now!

I'll be traveling over the next couple weeks, but I'll start on integrating this with Stellar's account viewer in early September and let you know if I run into anything.

[szymonlesisz]

I've also removed StellarGetPublicKey message.
Related to trezor/trezor-core@4305c1d

I think i can close this issue now since everything is working correctly

[zulucrypto]

Hi @szymonlesisz - I'm working on integrating this with Stellar's account viewer by importing https://connect.trezor.io/5/trezor-connect.js and building a custom firmware.

When I try to call TrezorConnect.stellarGetAddress it opens trezor connect but then gives me an error: "Firmware is not supported"

I'm getting the same error when I try using the connect explorer: https://trezor.github.io/connect-explorer/#/stellar-getaddress

[prusnak]

You need to set the firmware version to 1.7.0 for T1 (or use branch next from trezor-mcu) or 2.0.8 for T2 (or use master from trezor-core).

[zulucrypto]

@prusnak - Thanks for the quick response!

https://wallet.trezor.io reports that the device is running firmware 1.7.0 after I build the emulator with ./build-emulator.sh next

However, I'm still getting the same error. I dug through the code a little and saw this in the trezor connect source:

connect/src/js/core/methods/StellarGetAddress.js

Line 34 in bf0381f

this.requiredFirmware = ['0', '2.0.8'];

I've tried building trezor connect and connect-explorer locally to see if changing this parameter fixes anything, but I haven't had any luck getting them to talk to each other, so I want to double check with you that it's expected to work with requiredFirmware = ['0', '2.0.8'] before I go any further.

[szymonlesisz]

Hi @zulucrypto ,
Sorry, my bad. I was told that there will not be Stellar integration on Trezor1, that's why there is '0'.
If you running connect locally then change this line to requiredFirmware = ['1.7.0', '2.0.8'].
I'm changing this right away

[vlddm]

Can we have back stellarGetPublicKey please?
Corresponding method was removed from trezor-core trezor/trezor-core#325 as well as from trezor-connect aee28df but happens to exist in trezor-firmware so would be nice to have it in web again, need it for my application.

[prusnak]

@vlddm Just use GetPublicKey

[vlddm]

@vlddm Just use GetPublicKey

Thx, it works, but I bumped into another issue: looks like trezor using hardened path for stellar accounts, like m/44'/148'/0' for first address, m/44'/148'/1' for second and so on, so seems impossible to derive it from xpub and I have to use stellarGetAddress, which is not usable in my case (need to derive a lot of addresses).

Also seems like trezor does not support custom paths like m/44'/148'/0'/0/0 for Stellar tx signing.

Any ideas how can I use public keys for Stellar address deriving?

[prusnak]

@vlddm You are right. Please check this document: https://github.com/trezor/trezor-firmware/blob/master/docs/misc/coins-bip44-paths.md - you are supposed to use m/44'/148'/a' path where a is the account number. XPUBs are not applicable for Stellar as this uses ed25519 curve which does not have public BIP32 derivation. Your app needs to request public key for each account separately.