feat(protocol): [2] solana support by onur-ozkan · Pull Request #2622 · GLEECBTC/komodo-defi-framework

onur-ozkan · 2025-09-16T12:13:36Z

Implements withdraw and send_raw_transaction RPCs (covered with an integration test) for Solana coin and tokens along with some other minor parts.

Previous implementations:

Signed-off-by: Onur Özkan <work@onurozkan.dev>

onur-ozkan · 2025-09-18T05:17:10Z

mm2src/coins/tendermint/tendermint_coin.rs


 #[cfg(test)]
-pub mod tendermint_falsecoin_tests {
+pub mod tests {


This seems like was renamed unintentionally in #2223.

Signed-off-by: Onur Özkan <work@onurozkan.dev>

… into solana-withdraw

Signed-off-by: Onur Özkan <work@onurozkan.dev>

shamardy

Thanks for the PR! First review iteration from my side!

mm2src/coins/Cargo.toml

mm2src/coins/lp_coins.rs

mm2src/coins/solana/solana_coin.rs

mm2src/coins/solana/solana_token.rs

mm2src/coins/solana/solana_coin.rs

mm2src/coins/solana/solana_token.rs

shamardy · 2025-10-08T11:13:45Z

mm2src/coins/solana/solana_token.rs

+                instructions.push(create_associated_token_account(
+                    &coin.address,
+                    &to,
+                    &token.protocol_info.mint_address,
+                    &spl_token_program::id(),
+                ));


shouldn't we make sure we can cover the rent in such case? We should also account for the rent as part of the fee or add it to SolanaFeeDetails which makes more sense.

I don't know what the rent is. I need to learn it first.

I would've preferred it in this PR as it's related to withdraw, but it can be in a small PR next before transaction history.

mm2src/coins/solana/solana_token.rs

Signed-off-by: Onur Özkan <work@onurozkan.dev>

mariocynicys

awesome work!

first iter from my side, mostly questions xD

mm2src/coins_activation/src/solana_with_assets.rs

mm2src/coins/lp_coins.rs

mm2src/coins/solana/solana_coin.rs

mariocynicys · 2025-10-13T08:51:41Z

mm2src/coins/solana/solana_coin.rs

+            if lamports == 0 {
+                return MmError::err(WithdrawError::AmountTooLow {
+                    amount: req.amount,
+                    threshold: coin.min_tx_amount(),
+                });
+            }


it won't == 0, we do that check above in calculate-withdraw_amount (checking with coin.min_tx_amount())

Yes but it's hard to track (if we change calculate_withdraw_amount we will most likely forget to add the check in withdraw). So for the sake of safety/explicitly I would still keep it here to ensure everything is guaranteed.

mm2src/coins/solana/solana_coin.rs

mariocynicys · 2025-10-13T09:07:45Z

mm2src/coins/solana/solana_coin.rs

+            let fee = rpc
+                .get_fee_for_message(tx.message())
+                .map_err(|e| WithdrawError::Transport(e.to_string()))?;


~~cant we calculate the tx fee internally without invoking the rpc?~~
~~also im not sure if this method does an estimation or query the fee for this specific transaction from the blockchain?~~

nvm, i thought that solana_system_transaction::transfer sends the transaction on-chain.
but then i got a Q: does this mean that the tx fee is calculated dynamically based on the blockchain conditions? do we have any control/limit over the fees that could be spent from out account at the time of tx publishing?

Q: does this mean that the tx fee is calculated dynamically based on the blockchain conditions?

Yes.

Do we have any control/limit over the fees that could be spent from out account at the time of tx publishing?

Well, we see the fee before we broadcasting it, so there is a bit of control.

mariocynicys · 2025-10-13T10:05:38Z

mm2src/coins/solana/solana_token.rs

+    fn token_id(&self) -> RpcBytes {
+        sha256(self.ticker().to_lowercase().as_bytes()).to_vec().into()
+    }


don't we want to relate token_id to the mint address rather than the ticker, as the ticker might change, no?
or two tokens might have the same ticker?

I am not sure if it's ever possible to be 2 different tokens with the same ticker on the same chain. But I do liked using the mint address more.

mariocynicys · 2025-10-13T10:09:11Z

mm2src/coins/solana/solana_token.rs

+            if let Err(e) = rpc.get_account(&to_token_account) {
+                if !e.kind.to_string().contains("AccountNotFound") {
+                    return MmError::err(WithdrawError::Transport(e.to_string()));
+                }
+
+                instructions.push(create_associated_token_account(
+                    &coin.address,


Q: what happens if when publishing the tx we found out that the account has already been created? will the tx fail or this instruction get ignored?

I guess you are referring to the account being created between the transaction generation and the sending phase? In that case the transaction would fail. There is another function that allows it to succeed even if the account already exists, I can use that too if we don't want TX to fail.

mm2src/coins/solana/solana_token.rs

dimxy · 2025-10-13T18:25:51Z

mm2src/coins/solana/solana_token.rs

+
+            if let Err(e) = rpc.get_account(&to_token_account) {
+                if !e.kind.to_string().contains("AccountNotFound") {
+                    return MmError::err(WithdrawError::Transport(e.to_string()));


I'd like to remind that it's preferable to return error codes rather string messages,
especially when it is important to show errors translated into the user native language.
Here user probably would like to get a message in their native language that the destination account is not found?

I see in the KW repo they have a framework for error translation by error codes. So I think we should return more error codes.
(I guess we should return string descriptions (non-translatable) only when we get unrecoverable internal errors).

We are not sending custom string message here. The whole error converted into string so we can use it in WithdrawError. User will see the complete error message including the error code (if there are any) wrapped with KDF WithdrawError.

@onur-ozkan we can match the .kind field instead of serializing it and perform the contains() check.

It's not quite possible.

Signed-off-by: Onur Özkan <work@onurozkan.dev>

shamardy

I left some nits and things to do in a subsequent PR or here, whatever you prefer. Other than that, LGTM!

shamardy · 2025-10-23T05:49:27Z

mm2src/coins/solana/solana_coin.rs

    pub(crate) async fn rpc_client(&self) -> MmResult<Arc<RpcClient>, String> {
        let mut rpcs = self.rpc_clients.lock().await;

-        if let Some(index) = rpcs.iter().position(|rpc| rpc.get_health().is_ok()) {
-            // Put healthy one to the front.
-            rpcs.rotate_left(index);
-
-            return Ok(rpcs[0].clone());
+        for (index, rpc) in rpcs.iter().enumerate() {
+            if rpc.get_health().await.is_ok() {
+                rpcs.rotate_left(index);
+                return Ok(rpcs[0].clone());
+            }
        }


A comment for next PRs, please think about randomizing RPCs order on coin activation.

What do you mean? Why should we randomize RPCs order?

To not let all users use the first one and overload it, load should be distributed. At least it was done like that for EVMs. c.c. @cipig I would like his opinion on this.
But for GUI authenticated RPCs, we can have randomization as optional or we have a preferred primary RPC selected if needed.

To not let all users use the first one and overload it, load should be distributed. At least it was done like that for EVMs.

This would kill the ability of controlling the prioritization of the nodes we add to the application (e.g., having the paid node as the last backup and the most stable one as the primary/main node).

I understand this, we should also have a way to prioritize them into primary and secondary, there was an issue for this for other coins #1520
But all this is very low priority, so keep it as is for now.

shamardy · 2025-10-23T05:53:04Z

mm2src/coins/solana/solana_token.rs

+                instructions.push(create_associated_token_account(
+                    &coin.address,
+                    &to,
+                    &token.protocol_info.mint_address,
+                    &spl_token_program::id(),
+                ));


I would've preferred it in this PR as it's related to withdraw, but it can be in a small PR next before transaction history.

mm2src/coins/solana/solana_coin.rs

mm2src/mm2_main/tests/mm2_tests/solana_tests.rs

mm2src/coins/solana/solana_token.rs

shamardy · 2025-10-23T06:28:18Z

mm2src/coins/solana/solana_token.rs

+                    &coin.address,
+                    &to,
+                    &token.protocol_info.mint_address,
+                    &spl_token_program::id(),


spl_token_program::id() shouldn't be used IMHO, there is Token-2022 standard with different program id. Can be fixed in the rent followup PR as well. Not only here but in other places e.g. use get_associated_token_address_with_program_id instead of get_associated_token_address etc..

get_associated_token_address_with_program_id doesn't return program id, it takes it. I guess the best we can do is to put that value into the coins file in this case.

get_associated_token_address_with_program_id doesn't return program id, it takes it

I am aware, I mean to pass the right program id by using get_associated_token_address_with_program_id instead of get_associated_token_address

I guess the best we can do is to put that value into the coins file in this case.

There should be a token type / standard in coins file, as for imported tokens in the future, there are ways to detect which token standard it uses.

I think we can safely use them statically in the coins file without needing a dynamic detection logic, what do you think? Adding an extra field to coins file for tokens shouldn't do any harm. I made it here.

I think we can safely use them statically in the coins file without needing a dynamic detection logic, what do you think?

For Solana it's different and most wallets do dynamic detection if I am not mistaken, Phantom even detects any coin you have in your wallet that is not a scam / spam (their spam detection fails sometimes and have a way for users to mark tokens as spam). Please check industry standards for this and choose what is a right fit for Solana.

Signed-off-by: Onur Özkan <work@onurozkan.dev>

shamardy · 2025-10-24T11:26:27Z

mm2src/coins/solana/solana_token.rs

                }

+                rent_lamports = rpc
+                    .get_minimum_balance_for_rent_exemption(spl_token::state::Account::LEN)


I believe the length may differ for Token-2022 as they can add optional extensions however they like, so the rent exemption calculation will be a bit more complicated by checking each token for which extensions enabled, etc..

Added a TODO.

Signed-off-by: Onur Özkan <work@onurozkan.dev>

onur-ozkan added 2 commits September 16, 2025 08:22

implement send_raw_tx* functions for solana

d31f5bb

Signed-off-by: Onur Özkan <work@onurozkan.dev>

implement withdraw RPC for Solana

c3f52f2

Signed-off-by: Onur Özkan <work@onurozkan.dev>

onur-ozkan added status: in progress feature: protocol labels Sep 16, 2025

onur-ozkan added 8 commits September 17, 2025 07:26

handle solana fee details

dbe6033

Signed-off-by: Onur Özkan <work@onurozkan.dev>

add TODO

0b699ef

Signed-off-by: Onur Özkan <work@onurozkan.dev>

implement withdraw for solana tokens

751085c

Signed-off-by: Onur Özkan <work@onurozkan.dev>

disable enable-solana feature

e91089e

Signed-off-by: Onur Özkan <work@onurozkan.dev>

handle fee on received_by_me

076ca39

Signed-off-by: Onur Özkan <work@onurozkan.dev>

disable token check temporarily

4800d4e

Signed-off-by: Onur Özkan <work@onurozkan.dev>

rename test module for tendermint

3c7603a

Signed-off-by: Onur Özkan <work@onurozkan.dev>

add coverage on sol activation, withdraw and send_raw_transaction

ab1f3a5

Signed-off-by: Onur Özkan <work@onurozkan.dev>

onur-ozkan commented Sep 18, 2025

View reviewed changes

onur-ozkan added 7 commits September 18, 2025 08:33

feature gate solana_tests module

853db91

Signed-off-by: Onur Özkan <work@onurozkan.dev>

add another TODO

6748484

Signed-off-by: Onur Özkan <work@onurozkan.dev>

extend calculate_withdraw_amount

cc75fad

Signed-off-by: Onur Özkan <work@onurozkan.dev>

resolve solana_token todo

7094fcb

Signed-off-by: Onur Özkan <work@onurozkan.dev>

Merge branch 'dev' of github.com:KomodoPlatform/komodo-defi-framework…

ab565df

… into solana-withdraw

handle non-existent token accounts

903336d

Signed-off-by: Onur Özkan <work@onurozkan.dev>

handle token programs on token init

82d7a3a

Signed-off-by: Onur Özkan <work@onurozkan.dev>

onur-ozkan marked this pull request as ready for review October 7, 2025 09:05

onur-ozkan added status: pending review and removed status: in progress labels Oct 7, 2025

shamardy reviewed Oct 8, 2025

View reviewed changes

onur-ozkan added 4 commits October 9, 2025 08:25

bump spl-token

974ac57

Signed-off-by: Onur Özkan <work@onurozkan.dev>

use token transfer with proper token id

b13eb91

Signed-off-by: Onur Özkan <work@onurozkan.dev>

use token address on token transfer_checked

755e712

Signed-off-by: Onur Özkan <work@onurozkan.dev>

add missing items to untagged deser

3dbc4b8

Signed-off-by: Onur Özkan <work@onurozkan.dev>

onur-ozkan force-pushed the solana-withdraw branch from 72bbfea to 3dbc4b8 Compare October 13, 2025 09:44

mariocynicys reviewed Oct 13, 2025

View reviewed changes

dimxy reviewed Oct 13, 2025

View reviewed changes

mm2src/coins/solana/solana_token.rs Outdated Show resolved Hide resolved

dimxy reviewed Oct 13, 2025

View reviewed changes

minor improvements

84ac433

Signed-off-by: Onur Özkan <work@onurozkan.dev>

onur-ozkan force-pushed the solana-withdraw branch 3 times, most recently from 07c6762 to 4c39cb3 Compare October 16, 2025 08:06

update received_by_me and spent_by_me, my_balance_change

dd8eccf

Signed-off-by: Onur Özkan <work@onurozkan.dev>

onur-ozkan force-pushed the solana-withdraw branch from 4c39cb3 to dd8eccf Compare October 16, 2025 08:28

shamardy previously approved these changes Oct 23, 2025

View reviewed changes

some nit fixes

d41ea99

Signed-off-by: Onur Özkan <work@onurozkan.dev>

onur-ozkan dismissed shamardy’s stale review via d41ea99 October 23, 2025 08:02

shamardy mentioned this pull request Oct 23, 2025

Standardize how block_height and timestamp are used in transaction details #2664

Open

onur-ozkan added 3 commits October 23, 2025 12:02

better program id usage

dfe4a0e

Signed-off-by: Onur Özkan <work@onurozkan.dev>

re-write calculate_withdraw_and_fee_amount

6ec2ee7

Signed-off-by: Onur Özkan <work@onurozkan.dev>

handle rent amounts

e1d1582

Signed-off-by: Onur Özkan <work@onurozkan.dev>

shamardy reviewed Oct 24, 2025

View reviewed changes

shamardy added the 2.6.0-beta label Oct 24, 2025

add TODO

03b006c

Signed-off-by: Onur Özkan <work@onurozkan.dev>

onur-ozkan merged commit 9353a5d into dev Oct 27, 2025
19 of 25 checks passed

onur-ozkan deleted the solana-withdraw branch October 27, 2025 05:22

shamardy mentioned this pull request Nov 25, 2025

chore(release): v2.6.0-beta #2696

Merged

Conversation

onur-ozkan commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shamardy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mariocynicys left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shamardy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

onur-ozkan Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

onur-ozkan commented Sep 16, 2025 •

edited

Loading

onur-ozkan Oct 23, 2025 •

edited

Loading

shamardy Oct 23, 2025 •

edited

Loading