search

OmniLayer / omniwallet

Omni Protocol Hybrid Web-Wallet
https://www.omniwallet.org
GNU Affero General Public License v3.0
327 stars 187 forks source link

Trezor support #1694

Open prusnak opened 5 years ago

prusnak commented 5 years ago

Since the new firmware (1.7.2 and 2.0.10) Trezor does support Omni layer. It should be pretty straightforward to get Trezor working in Omni using Trezor Connect: http://github.com/trezor/connect

boonlannis commented 5 years ago

Really excited to see some progress made towards integrating Omni within the Trezor wallet! I'm sure there are plenty of use cases but I know the community over at MaidSafe are excited to see this as well. See the thread here: https://safenetforum.org/t/trezor-support-for-omni-and-main/26816/8 and here: https://safenetforum.org/t/trezor-and-maidsafecoin/18850/19 and here: https://safenetforum.org/t/sending-maid-from-a-trezor-hardware-wallet/23783/55

Thanks for all the hardwork!

DavidMc0 commented 5 years ago

After seeing work on hardware wallet support mentioned numerous times on Omni's state of the layer updates for months / years, its a shame they haven't kept up with Trezor on supporting this feature as soon as Trezor implemented it.

Looking forward to a working Omniwallet for Trezor, and any updates about the priority this will be given / time frames if possible.

achamely commented 5 years ago

@prusnak yup, we saw the updates and are working internally on this and should have something in the next few weeks

ghost commented 5 years ago

Just updating this thread to show the interest of the community in having this feature implemented. Keep up with the hard work! :+1:

rid-dim commented 5 years ago

Hmm - any updates on this topic?

achamely commented 5 years ago

Still in progress internally

prusnak commented 5 years ago

@achamely Why don't you develop in the open? This is not how a project that claims to be open-source should be developing things.

achamely commented 5 years ago

@prusnak when i say still in progress internally i mean there is nothing yet ready for public display. We are working on an integration that is testable/usable. Once code is ready for public use it will be published openly in the repo

kwests commented 5 years ago

Hope to see some progress soon.

rid-dim commented 5 years ago

?

prusnak commented 5 years ago

@achamely can you update the situation about the integration you are supposedly working on?

kwests commented 5 years ago

would be great to hear from you. 👍

boonlannis commented 5 years ago

Hi Team - any updates on this? I have to admit it is a bit disappointing as the first update from @achamely stated they may have an update within a few weeks, yet that was back in January. Curious if the issue is technical in nature, lack of funds, competing priorities, or (I'm sure) a combination of the three. I do believe some additional communication could allow the community to help wherever needed. Thank you!

zeiv commented 5 years ago

Hey all, I went ahead and put together https://omnitrezor.com/. It's a tool that can be used to create and send Simple Send OMNI transactions using your Trezor's Bitcoin accounts. In other words, you can use it to send and receive and OMNI coin (USDT, MAID, etc) with your Trezor. I've done a dozen or so live test transactions and all of them have gone through. It was a bit more complicated than I expected, but still took about a couple days.

@ OMNI, lest there be any misunderstanding, I do not intend to call anybody out here. I understand that resources are limited, priorities change, and sometimes important projects still end up being shifted to the back burner. I didn't spend a massive amount of time on this tool since I wasn't sure when the official Trezor integration will be out. If it will still be a while, can you let us know? Because in that case I might want to improve the UX, etc.

@ Everyone else, pull requests and other feedback are most welcome! I recommend you send a small test transaction first before sending your full amount, just to be safe. omnitrezor.com is open source and available on https://github.com/tektite-software/omni-trezor.

TL;DR - Sending OMNI transactions now possible via https://omnitrezor.com

boonlannis commented 5 years ago

Thank you @zeiv ! I haven’t tried this tool out yet but looks like exactly what is needed.

achamely commented 5 years ago

@zeiv thank you for putting that together. We are still working to develop a more full fledged setup but yes, there have been some resource constraints causing priority adjustments over the past few months that have unfortunately delayed this.

One small thing to note is that only divisible token amounts need to be adjusted/converted https://github.com/tektite-software/omni-trezor/blob/master/src/App.js#L242 indivisible tokens (like Maidsafe, should not be adjusted like this as it can send the wrong amount, off by an increased factor of 1e8) , details https://github.com/OmniLayer/spec#field-number-of-coins

zeiv commented 5 years ago

@achamely, wow, thanks for the heads up. I've deployed a fix for the amount encoding on non-divisible tokens. Interestingly, the same issue is present on the Trezor's firmware itself. Here's an image from testing before the fix:

IMG_0096

I went ahead and made the OP_RETURN preview toggleable so people are able to inspect the values more easily. I also double checked the relevant areas of the spec. Here is the updated version with the transaction amount fix. Note how the Trezor incorrectly interprets the amount as divisible.

IMG_0097

But you can see on the transaction everything is as expected: https://omniexplorer.info/tx/ecc9377e55bd950ee73aee38a103beaae217666b115e25d17c718a1aae82341e

prusnak commented 5 years ago

I'll fix the bug in Trezor in the next firmware update. So are there only two options?

1) token is not divisible - do not translate 2) token is divisible - translate by 1e8

Correct?

prusnak commented 5 years ago

@zeiv I fixed the Trezor firmware via https://github.com/trezor/trezor-firmware/commit/c288514a4fcdb528ac637eb29ba711c367008c66 Can you confirm the fix works for you? (You can use the Trezor emulator so you don't need to re-flash your device)

achamely commented 5 years ago

@prusnak that is correct, Divisible tokens can be sent in fraction amounts (i.e. 1.1, 0.3, 45.222444) So when encoding them into the opreturn they are translated by 1e8 for proper encoding. Indivisible tokens can only be sent in whole amounts (1,2,3,4,5,6, ... ,4324222) so they do not need to be translated.

rid-dim commented 5 years ago

Okay - for the record (I guess @prusnak knows it anyway - but maybe for others following this issue) there is a third party wallet now supporting omni token on the trezor https://trezor.io/coins/ - doesn't mean you it wouldn't be cool if omni supports it Natively but There seems to be a workaround now (didn't look into it myself yet and no clue how trustworthy/easy to use it is.. )