Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow Solana AToken Create instruction to contain an optional rent_sysvar account #3976

Closed

Conversation

gabrielKerekes
Copy link
Contributor

@gabrielKerekes gabrielKerekes commented Jun 25, 2024

This is needed because some dApps (https://jup.ag/perps or http://solend.fi/) include the rent_sysvar account although it's not required anymore (it's also not a part of the official docs). Trezor followed the official docs and didn't support this "legacy" format.

I've tested submitting a tx via https://jup.ag/perps with the updated Trezor and it went through.

I'm unable to update the ui test fixtures as I am not able to build the FW on the current main branch for some reason (I've developed this on top of a random older commit - 12725d98358f6fcb31a04b9a2b1b443e7f359a09).

Here's the last line from my build output which produces the error
gcc -o build/unix/embed/lib/mini_printf.o -c -DSCM_REVISION='"\x95\xc2\xcb\x20\x08\xa2\x98\xd4\xac\x78\x97\x7b\xb4\x3b\x43\x7d\x51\x6a\x7a\xbf"' -DCONFIDENTIAL= -DPYOPT=0 -DBITCOIN_ONLY=0 -DSTATIC= -Os -g3 -std=gnu11 -Wall -Werror -Wuninitialized -Wno-missing-braces -fdata-sections -ffunction-sections -fPIE -Wno-sequence-point -DTREZOR_EMULATOR -DTREZOR_MODEL_T -DMP_CONFIGFILE=\"embed/unix/mpconfigport.h\" -DUI_LAYOUT_TT -DAES_128 -DAES_192 -DUSE_INSECURE_PRNG -DUSE_BIP32_CACHE=0 -DUSE_KECCAK=1 -DUSE_ETHEREUM=1 -DUSE_MONERO=1 -DUSE_CARDANO=1 -DUSE_NEM=1 -DUSE_EOS=1 -DUSE_SECP256K1_ZKP -DUSE_SECP256K1_ZKP_ECDSA -DSECP256K1_CONTEXT_SIZE=208 -DUSE_EXTERNAL_DEFAULT_CALLBACKS -DECMULT_GEN_PREC_BITS=2 -DECMULT_WINDOW_SIZE=8 -DENABLE_MODULE_GENERATOR -DENABLE_MODULE_RECOVERY -DENABLE_MODULE_SCHNORRSIG -DENABLE_MODULE_EXTRAKEYS -DUSE_AES_GCM -DAES_VAR -DTREZOR_UI2 -DTRANSLATIONS -DFANCY_FATAL_ERROR -DTREZOR_FONT_NORMAL_ENABLE=Font_TTHoves_Regular_21 -DTREZOR_FONT_NORMAL_INCLUDE=\"font_tthoves_regular_21.h\" -DTREZOR_FONT_DEMIBOLD_ENABLE=Font_TTHoves_DemiBold_21 -DTREZOR_FONT_DEMIBOLD_INCLUDE=\"font_tthoves_demibold_21.h\" -DTREZOR_FONT_MONO_ENABLE=Font_RobotoMono_Medium_20 -DTREZOR_FONT_MONO_INCLUDE=\"font_robotomono_medium_20.h\" -DTREZOR_FONT_BOLD_UPPER_ENABLE=Font_TTHoves_Bold_17_upper -DTREZOR_FONT_BOLD_UPPER_INCLUDE=\"font_tthoves_bold_17.h\" -DSTM32F427xx -DTREZOR_BOARD=\"T2T1/boards/t2t1-unix.h\" -DHW_MODEL=827601492 -DHW_REVISION=0 -DMCU_TYPE=STM32F427xx -DFLASH_BIT_ACCESS=1 -DFLASH_BLOCK_WORDS=1 -DUSE_DMA2D -D_THREAD_SAFE -Ibuild/unix -I. -Ibuild/unix/embed/rust -Iembed/rust -Ibuild/unix/embed/lib -Iembed/lib -Ibuild/unix/embed/models -Iembed/models -Ibuild/unix/embed/unix -Iembed/unix -Ibuild/unix/embed/trezorhal -Iembed/trezorhal -Ibuild/unix/embed/trezorhal/unix -Iembed/trezorhal/unix -Ibuild/unix/embed/extmod/modtrezorui -Iembed/extmod/modtrezorui -Ibuild/unix/vendor/micropython -Ivendor/micropython -Ibuild/unix/vendor/micropython/ports/unix -Ivendor/micropython/ports/unix -Ibuild/unix/vendor/micropython/lib/mp-readline -Ivendor/micropython/lib/mp-readline -Ibuild/unix/embed/extmod/modtrezorconfig -Iembed/extmod/modtrezorconfig -Ibuild/unix/vendor/trezor-storage -Ivendor/trezor-storage -Ibuild/unix/vendor/trezor-crypto -Ivendor/trezor-crypto -Ibuild/unix/vendor/secp256k1-zkp -Ivendor/secp256k1-zkp -Ibuild/unix/vendor/secp256k1-zkp/src -Ivendor/secp256k1-zkp/src -Ibuild/unix/vendor/secp256k1-zkp/include -Ivendor/secp256k1-zkp/include -Ibuild/unix/vendor/micropython/lib/uzlib -Ivendor/micropython/lib/uzlib -I/nix/store/n4ry38iwq6y9wy7fpal3qpw7kzq0f1l1-SDL2-2.26.4-dev/include -I/nix/store/n4ry38iwq6y9wy7fpal3qpw7kzq0f1l1-SDL2-2.26.4-dev/include/SDL2 -I/nix/store/55ymc7rb2imvv0wslf9g8hv1g3h3fnmq-SDL2_image-2.6.3/include/SDL2 embed/lib/mini_printf.c
embed/lib/mini_printf.c:110:2: error: function definition is not allowed here
        {
        ^
embed/lib/mini_printf.c:119:2: error: function definition is not allowed here
        {
        ^
embed/lib/mini_printf.c:137:4: error: call to undeclared function '_putc'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
                        _putc(ch);
                        ^
embed/lib/mini_printf.c:162:6: error: call to undeclared function '_puts'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
                                        _puts(bf, len);
                                        ^
4 errors generated.
scons: *** [build/unix/embed/lib/mini_printf.o] Error 1

…rent_sysvar` account

This is needed because some dApps (https://jup.ag/perps or http://solend.fi/) include the rent_sysvar account although it's not required anymore (it's also not a part of the [official docs](https://docs.rs/spl-associated-token-account/latest/spl_associated_token_account/instruction/enum.AssociatedTokenAccountInstruction.html#variant.Create)).
@gabrielKerekes gabrielKerekes requested a review from matejcik as a code owner June 25, 2024 08:57
@Hannsek
Copy link
Contributor

Hannsek commented Jun 25, 2024

So, shouldn't be a "fix" on their side? This is additional code for us…

@gabrielKerekes
Copy link
Contributor Author

So, shouldn't be a "fix" on their side?

Possibly. Although there might be more dApps/wallets which include the rent sysvar and expecting them all to update might not be feasible. Although so far there have been only two reports of this kind (at least AFAIK).

@Hannsek
Copy link
Contributor

Hannsek commented Jun 25, 2024

Just to understand it better, you cannot sign any transaction without it with affected wallets?

@gabrielKerekes
Copy link
Contributor Author

Just to understand it better, you cannot sign any transaction without it with affected wallets?

What do you mean by the "it" in "without it" and by "affected wallets"?

Without this fix Trezor users won't be able to use some dApps which require token accounts and create them via the associated token accounts program with the "legacy" instruction format. You might still be able to use some parts of a dApp, which don't require an AToken account.

Also if a wallet uses the legacy format then Trezor users won't be able to sign transactions which create token accounts e.g. transferring tokens to an address which doesn't own that token yet. However since there aren't many wallets with Trezor integration, this shouldn't be a problem and a new wallet integrating Trezor should be able to fix this on their end.

@matejcik matejcik added this to the June release milestone Jun 25, 2024
@Hannsek
Copy link
Contributor

Hannsek commented Jun 25, 2024

What do you mean by the "it" in "without it" and by "affected wallets"?

I meant rent_sysvar account and by "affected wallets" I meant those who still use rent_sysvar accounts

@matejcik
Copy link
Contributor

matejcik commented Jul 1, 2024

included via #3984

@matejcik matejcik closed this Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants