Solana #1109

Milerius · 2021-10-15T15:38:15Z

This is my first attempt to implement a protocol at AtomicDEX, there may be a lot of errors.

Example of conf:

{
      "coin": "TSOL",
      "name": "solana testnet",
      "fname": "Solana testnet",
      "mm2": 1,
      "decimals": 8,
      "wallet_only": true,
      "protocol": {
        "type": "SOLANA"
      }
  },

enable request:

{
    "coin": "TSOL",
    "commitment_level": "Finalized",
    "method": "enable",
    "tx_history": false,
    "urls": [
        "https://api.testnet.solana.com"
    ],
    "userpass": "simplepassword123"
}

enable answer:

 {
        "result": "success",
        "address": "FJktmyjV9aBHEShT4hfnLpr9ELywdwVtEL1w1rSWgbVf",
        "balance": "1.999860000000000",
        "unspendable_balance": "0",
        "coin": "TSOL",
        "required_confirmations": 1,
        "requires_notarization": false
}

Some information before reviewing

Solana doesn't use a number of confirmations but instead use a commitment level https://explorer.solana.com/tx/41XkzDsUKr9xE5JWCnLqYGBUUCsZqEpDaAc2fp1HuVwn926PXC81VfqgE2FxTo6UGSFdDWiGDai8J5zYrfHTS3Ke?cluster=testnet
I've used 1 confirmation in settings meanwhile we figure out how to use this commitment level instead of the number of confirmations
My first idea would be to map this u64 to a commitment level (1 = Processed, 2 = Confirmed, 3 = Finalized)
I have a proof of concept for tx_history that work for Solana coin and it uses the file solana_decode_tx_helpers.rs
Solana doesn't use a transaction hex hash but instead, it's a String e.g 41XkzDsUKr9xE5JWCnLqYGBUUCsZqEpDaAc2fp1HuVwn926PXC81VfqgE2FxTo6UGSFdDWiGDai8J5zYrfHTS3Ke (2 solutions for that I transformed tx_hash to be a String.
By default Solana support Multi-Account, but internally atm we use account[0] before [feature request] HD Wallet (multi-account) #740 is implemented: https://github.com/KomodoPlatform/atomicDEX-API/blob/4ab8e4c089cead9a9f3ec9d685306d1dca6e2789/mm2src/coins/solana/spl.rs#L148-L158
For the moment Iguana Seed secret slice is used to generate the Dalek private key, but in the unit test, there is a proof of concept that shows based on a valid bip39 seed how to retrieve the same public address as hardware wallet (ledger) or trust wallet: https://github.com/KomodoPlatform/atomicDEX-API/blob/4ab8e4c089cead9a9f3ec9d685306d1dca6e2789/mm2src/coins/solana/solana_common_tests.rs#L24-L42
withdraw for SLP tokens use the instruction transfer_checked because this one gives more information when parsing transaction history and simplify the generation of transaction_details.
I've some duplicated code for withdraw prerequisites for both SPL and Solana, would be nice if I can write a common function that can be used from both sides.

What's implemented:

my_balance: ✅
base_coin_balance: ✅
withdraw: ✅
current_block: ✅
validate_address: ✅
decimals: ✅
is_asset_chain: ✅
display_priv_key: ✅
my_address: ✅
ticker: ✅
send_raw_tx: ✅

- Add a unit tests for the basis regarding the wallet - Add a skeleton SolanaCoin and SolanaCoinImpl that compiles - Had to modify one of the dependencies to compile locally, CI will not pass - Add base58 in the crate - Add ed25519 helpers - Add solana-sdk and solana-client 1.7.12

- Implement my_address()

…into solana

…n on solana

# Conflicts: # Cargo.lock # mm2src/mm2_libp2p/Cargo.toml

sergeyboyko0791 · 2022-03-23T10:56:17Z

mm2src/coins/solana/solana_common.rs

+    let sol_required = lamports_to_sol(fees);
+    if base_balance < sol_required {
+        return MmError::err(SufficientBalanceError::NotSufficientBalance {
+            coin: coin.ticker().to_string(),


I think we should add a method like base_coin_ticker to either MarketCoinOps or SolanaCommonOps, because we check the balance of the base coin.
I wanted to add this method to MarketCoinOps, but don't remember why I haven't done it.
Could you please investigate if we can add MarketCoinOps::base_coin_ticker, and if it's impossible, add SolanaCommonOps::base_coin_ticker.
It should work like MarketCoinOps::base_coin_balance.

sergeyboyko0791 · 2022-03-23T10:59:29Z

mm2src/coins/solana/solana_common.rs

+        return MmError::err(SufficientBalanceError::NotSufficientBalance {
+            coin: coin.ticker().to_string(),
+            available: base_balance.clone(),
+            required: &sol_required - &base_balance,


NotSufficientBalance::required means the threshold, the minimum amount that we must have to perform this transfer.
So it should be required: sol_required.

sergeyboyko0791 · 2022-03-23T11:03:54Z

mm2src/coins/solana/solana_common.rs

+    if to_send < sol_required && !coin.is_token() {
+        return MmError::err(SufficientBalanceError::AmountTooLow {
+            amount: to_send.clone(),
+            threshold: &sol_required - &to_send,


Here the same :)

sergeyboyko0791 · 2022-03-23T11:07:06Z

mm2src/coins/solana/solana_common.rs

+        return MmError::err(SufficientBalanceError::NotSufficientBalance {
+            coin: coin.ticker().to_string(),
+            available: my_balance.clone(),
+            required: &to_check - &my_balance,


sergeyboyko0791 · 2022-03-23T11:45:44Z

mm2src/coins/solana/solana_tests.rs

+        let expected_spent_by_me = &request_amount + &sol_required;
+        assert_eq!(valid_tx_details.spent_by_me, expected_spent_by_me);
+        assert_eq!(valid_tx_details.received_by_me, request_amount);
+        assert_eq!(valid_tx_details.total_amount, request_amount);


If I understood correctly, TransactionDetails::total_amount has to be equal to TransactionDetails::spent_by_me:

For UTXO, total_amount is a sum of inputs
https://github.com/KomodoPlatform/atomicDEX-API/blob/03b0910362709eae9c2c0124932bfc295692da94/mm2src/coins/utxo/utxo_common.rs#L2430

For ETH, it's equal to spent_by_me:
https://github.com/KomodoPlatform/atomicDEX-API/blob/03b0910362709eae9c2c0124932bfc295692da94/mm2src/coins/eth.rs#L629-L643

sergeyboyko0791 · 2022-03-23T11:52:00Z

mm2src/coins/solana/solana_tests.rs

+            valid_tx_details.received_by_me,
+            BigDecimal::from_str("0.000095").unwrap()
+        );
+        assert_eq!(valid_tx_details.total_amount, BigDecimal::from_str("0.000105").unwrap());


This conclusion is wrong. total_amount should equal to spent_by_me

sergeyboyko0791

A few more changes please :)

sergeyboyko0791

Last few things :)

sergeyboyko0791 · 2022-03-24T07:38:03Z

mm2src/coins/lightning.rs

@@ -387,6 +387,8 @@ impl MarketCoinOps for LightningCoin {
        Box::new(self.platform_coin().my_balance().map(|res| res.spendable))
    }

+    fn base_coin_ticker(&self) -> &str { self.ticker() }


I'm not familiar with Lightning integration, but I think the ticker of LightningCoin may be different from its base coin.
@shamardy could you please confirm or deny this?

I'm not familiar with Lightning integration, but I think the ticker of LightningCoin may be different from its base coin.

This is correct. this should be self.platform_coin().ticker()

sergeyboyko0791 · 2022-03-24T07:44:43Z

mm2src/coins/test_coin.rs

@@ -39,6 +39,8 @@ impl MarketCoinOps for TestCoin {

    fn base_coin_balance(&self) -> BalanceFut<BigDecimal> { unimplemented!() }

+    fn base_coin_ticker(&self) -> &str { self.ticker() }


This can be simplified as unimplemented :)

…nted

sergeyboyko0791

Thanks for the fixes! LGTM 🔥

shamardy

Only one non-blocker comment

mm2src/coins/utxo/slp.rs

…into solana

sergeyboyko0791

Approve

artemii235

🔥

Milerius added 30 commits September 25, 2021 10:00

feat(solana): continue a solana implementation

96f9bf6

- Implement my_address()

feat(solana): add my_balance support

d205245

feat(solana): add spl program id

501d3f8

feat(solana): update Cargo.toml

68b1c63

Merge branch 'dev' of https://github.com/KomodoPlatform/atomicDEX-API …

471f0db

…into solana

feat(solana): add current block and my_balance for tokens

4f58d68

feat(solana): unit test for custom key

823ce2a

feat(solana): polish unit test

24855cd

feat(solana): use a finalized commitment level

f54eb0a

feat(solana_tests): use bobpassphrase now

10e91a7

feat(solana): add validate address

9085e2d

feat(solana): add is_on_curve for validate address

1156d19

feat(solana): better address validation

4b9737a

feat(solana): simplify unit test by choosing the network

ed78b99

feat(solana): comment a simple transaction example

926c220

feat(solana): use bincode to serialize,deserialize a tx

d7568b3

feat(solana): implement withdraw for base coin

1aaad18

feat(solana): more unit tests for withdraw

d4b0a02

feat(solana): add a new unit test for spl transfer

60be601

feat(solana): start withdraw_spl_token_impl

504ec0b

feat(solana): add spl transfer

7f85318

feat(solana): add proof of concept for tx history

9bbc7ae

feat(solana): add data structure that represent a transfer transactio…

1642440

…n on solana

feat(solana): use last version libp2p

0df2bab

feat(solana): use last version libp2p

b2cf293

Merge remote-tracking branch 'origin/solana' into solana

9561c6a

# Conflicts: # Cargo.lock # mm2src/mm2_libp2p/Cargo.toml

feat(cargo): revert last commit

09c50ae

feat(solana): update cargo lock

fe4b368

feat(p2p): update step by step atomicdex libp2p

0472ff7

Milerius added 5 commits March 21, 2022 07:14

feat(dev): sync with dev

feba5c2

feat(solana): improve balance checking and better AmountTooLow case

1308152

feat(solana): move check amount to low, after calculating to_send

f51b95c

feat(solana): better unit tests, review fixes

0b47eb6

feat(solana): another batch of review fixes

2a48321

sergeyboyko0791 reviewed Mar 23, 2022

View reviewed changes

sergeyboyko0791 suggested changes Mar 23, 2022

View reviewed changes

Milerius added 5 commits March 23, 2022 13:02

feat(dev): sync with dev

3dba1d4

feat(solana): first batch of fixes

823fbbb

feat(solana): second batch of fixes

164815d

feat(solana): second batch of fixes

668c4e7

feat(solana): remove amount too low check

e5765bb

sergeyboyko0791 reviewed Mar 24, 2022

View reviewed changes

feat(base_coin): appropriate base_coin for ln and test_coin unimpleme…

fffd7c7

…nted

sergeyboyko0791 previously approved these changes Mar 25, 2022

View reviewed changes

feat(wasm): downgrade wasm-bindgen

683d441

Milerius dismissed sergeyboyko0791’s stale review via 683d441 March 26, 2022 06:19

Milerius requested review from sergeyboyko0791 and shamardy March 29, 2022 03:29

shamardy reviewed Mar 29, 2022

View reviewed changes

mm2src/coins/utxo/slp.rs Outdated Show resolved Hide resolved

Milerius added 4 commits March 29, 2022 10:17

Merge branch 'dev' of https://github.com/KomodoPlatform/atomicDEX-API …

6467790

…into solana

feat(solana): fixes review

20f94d5

feat(solana): fix erroneous rename from CLion

7791c17

feat(solana): delete platform_ticker from slp implementation

00995ed

sergeyboyko0791 approved these changes Mar 29, 2022

View reviewed changes

shamardy approved these changes Mar 29, 2022

View reviewed changes

artemii235 approved these changes Mar 31, 2022

View reviewed changes

artemii235 merged commit 86c27f7 into dev Mar 31, 2022

artemii235 deleted the solana branch March 31, 2022 12:14

tonymorony mentioned this pull request Apr 10, 2022

[FEATURE REQUEST]: Solana adding KomodoPlatform/komodo-wallet-desktop#1738

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Solana #1109

Solana #1109

Milerius commented Oct 15, 2021 •

edited

sergeyboyko0791 Mar 23, 2022

sergeyboyko0791 Mar 23, 2022

sergeyboyko0791 Mar 23, 2022

sergeyboyko0791 Mar 23, 2022

sergeyboyko0791 Mar 23, 2022

sergeyboyko0791 Mar 23, 2022

sergeyboyko0791 left a comment

sergeyboyko0791 left a comment

sergeyboyko0791 Mar 24, 2022

shamardy Mar 25, 2022

sergeyboyko0791 Mar 24, 2022

sergeyboyko0791 left a comment

shamardy left a comment

sergeyboyko0791 left a comment

artemii235 left a comment

		@@ -39,6 +39,8 @@ impl MarketCoinOps for TestCoin {

		fn base_coin_balance(&self) -> BalanceFut<BigDecimal> { unimplemented!() }

		fn base_coin_ticker(&self) -> &str { self.ticker() }

Solana #1109

Solana #1109

Conversation

Milerius commented Oct 15, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sergeyboyko0791 left a comment

Choose a reason for hiding this comment

sergeyboyko0791 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sergeyboyko0791 left a comment

Choose a reason for hiding this comment

shamardy left a comment

Choose a reason for hiding this comment

sergeyboyko0791 left a comment

Choose a reason for hiding this comment

artemii235 left a comment

Choose a reason for hiding this comment

Milerius commented Oct 15, 2021 •

edited