trezor-firmware
trezor-firmware copied to clipboard
Support `USE_{SOME COIN}` compilation flags
Compilation flags of type USE_* are not separable, i.e. they can be all disabled (by BITCOIN_ONLY=1), or all enabled (otherwise), but they cannot be set individually.
For example for USE_ETHEREUM=0 I get this error
In file included from embed/extmod/modtrezorcrypto/modtrezorcrypto.c:40:
embed/extmod/modtrezorcrypto/modtrezorcrypto-bip32.h: In function 'mod_trezorcrypto_HDNode_ethereum_pubkeyhash':
embed/extmod/modtrezorcrypto/modtrezorcrypto-bip32.h:437:3: error: implicit declaration of function 'hdnode_get_ethereum_pubkeyhash' [-Werror=implicit-function-declaration]
437 | hdnode_get_ethereum_pubkeyhash(&o->hdnode, (uint8_t *)pkh.buf);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Compilation flags will play a critical role in getting new coins and features into overflowing flash memory.
To be specific I need to disable some coins to push Zcash shielded transaction into flash.
The USE_ETHEREUM, USE_MONERO, USE_CARDANO, USE_NEM and USE_EOS are read only by trezor-crypto build, whereas the firmware build reads the BITCOIN_ONLY flag. Thus when one unsets the USE_ETHEREUM and BITCOIN_ONLY flags, the firmware will build with Ethereum but trezor-crypto will build without it, resulting in the error we see in the OP. It would seem to make a lot of sense to unify this in favor of the first set of flags which are additive, see https://github.com/trezor/trezor-firmware/issues/2370, and get rid of the BITCOIN_ONLY flag in code. In that case it would also make sense to add the following flags: USE_BINANCE, USE_BITCOINLIKE, USE_RIPPLE, USE_STELLAR, USE_TEZOS, USE_WEBAUTHN, USE_ZCASH.
What do you think @matejcik?
Actually we should ideally be distinguishing between Zcash shielded and unshielded, because the memory overhead of shielded transactions appears to be overwhelming. So USE_BITCOINLIKE (including Zcash unshielded) vs. USE_ZCASH_SHIELDED.
I don't see the need for per-coin use flags, there is no plan to mix-and-match.
We can flip "bitcoin-only" to something like "USE_ALL_FEATURES", or optionally "USE_ALTCOINS" and "USE_WEBAUTHN", and then add "USE_ZCASH_SHIELDED" (which also implies inclusion of Zcash unshielded)
I don't see the need for per-coin use flags, there is no plan to mix-and-match.
True, what I am proposing is very fine-grained. However, considering that we are running out of flash memory, we will have to resort to creating specialized firmwares, such as the one for Zcash-shielded transactions. I imagine this would then be helpful in creating thematic firmwares, such as "privacy firmware" with BTC + Zcash (shielded) + Monero or something similar.
we will have to resort to creating specialized firmwares
I am not yet convinced that "specialized firmwares" are a sensible way forward here. Some sort of pluggable architecture ala Ledger is an equally convincing alternative.
that said, I generally agree with you but I wouldn't go ahead with the fine-grained control until we actually start using it.
A pluggable architecture would be a nice solution, but we are very far from being able to implement that, whereas specialized firmwares is something that we are already doing. Anyway, I can understand the hesitation around prematurely implementing fine-grained control, so let's deal with this as we go and focus effort on https://github.com/trezor/trezor-firmware/issues/2370.
definitely +1 to being able to switch in and out features as required (even as an interim measure), the flash size constraint has been a real challenge for #2372, particularly because we do need to be able to test -each- coin (and will going forward also need to include our own)
(the version of this i started in case it is of use to y'all: per-coin-flags.txt, though there are still a lot of places to propagate these changes)