trezor-suite
trezor-suite copied to clipboard
Move trezor-address-validator to monorepo
https://github.com/trezor/trezor-address-validator
We don't need to keep it as fork, it's already heavy customized for our needs.
- [ ] move to monorepo
- [ ] convert to TS?
- [ ] publish as @trezor/trezor-address-validator under trezor-org
- [ ] use the new package in Suite
- [x] add BTC regtest support https://github.com/trezor/trezor-suite/pull/5786 via https://github.com/trezor/trezor-address-validator/pull/18
- [ ] fix validation of few address types in connect examples which should be valid: BTC BC1SW50QA3JX3S BTC bc1pw508d6qejxtdg4y5r3zarvary0c5xw7kw508d6qejxtdg4y5r3zarvary0c5xw7k7grplx
cc @martinboehm @mroz22 @szymonlesisz
- fix validation of few address types in connect examples which should be valid: BTC BC1SW50QA3JX3S BTC bc1pw508d6qejxtdg4y5r3zarvary0c5xw7kw508d6qejxtdg4y5r3zarvary0c5xw7k7grplx
Where did this requirement come from? I think that Trezor won't sign a transaction with these output addresses, see [1], [2].
bc1pw508d6qejxtdg4y5r3zarvary0c5xw7kw508d6qejxtdg4y5r3zarvary0c5xw7k7grplx This is SegWit v1 (Taproot), but it's using Bech32 encoding, not Bech32m. That seems to be in violation of BIP 350. So it wouldn't pass the code checks in [1].
BC1SW50QA3JX3S This one has the same problem and in addition it seems to be version 16. So even if it was Bech32m-encoded, it wouldn't pass the code checks in [2].
The reason why we have these checks is so that we don't send the user's assets to an address which Trezor does not understand, because that could lead to the assets being unspendable if Trezor doesn't encode the address correctly. Something similar happened to Binance after Taproot activation [3]. Binance failed terribly in this case, because they declared Taproot support but didn't care to run it through QA.
[1] https://github.com/trezor/trezor-firmware/blob/fb68db4302faea8b773dc61fe7241af7c6f6f4aa/core/src/trezor/crypto/bech32.py#L157-L183 [2] https://github.com/trezor/trezor-firmware/blob/fb68db4302faea8b773dc61fe7241af7c6f6f4aa/core/src/apps/bitcoin/common.py#L130-L139 [3] https://www.reddit.com/r/CryptoCurrency/comments/rp1ly7/binance_lost_users_bitcoin_because_they_converted/
similar checks are in connect > utxo-lib validation with the difference that version 1+ of Bech32m are supported.
maybe suite send form should have additional check: if it's taproot address and address version is supported by FW else show meaningful error
in other case send form will validate address as correct yet signing tx process will fail anyway
- fix validation of few address types in connect examples which should be valid: BTC BC1SW50QA3JX3S BTC bc1pw508d6qejxtdg4y5r3zarvary0c5xw7kw508d6qejxtdg4y5r3zarvary0c5xw7k7grplx
Where did this requirement come from?
I just copied that from https://github.com/trezor/trezor-suite/issues/4871
Hi @matejkriz, I just released a new version of trezor-address-validator, which includes the regtest modifications done by @szymonlesisz and Monero subaddress fixes done by Vlad.
I think this should be the last version outside of the monorepo. If you give me a hint how to do it correctly, I could create a PR which will do the switch to monorepo.
Thanks for your offer @martinboehm! I guess it's up to @mroz22 to move it. It's about creating a new package in monorepo and putting there the code from current trezor-address-validator, but converted to typescript and setting up CI jobs to publish it as a new package.