-
Notifications
You must be signed in to change notification settings - Fork 94
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
test(solana): migrate to local/offline containerized node #2168
Conversation
@0xMMBD please fix conflicts and fmt, lint checks |
Hello that's interesting that lint failed, I'll take a look at it (as well as the conflict). |
When it comes to the tests unfortunately I'm not sure why some of them are failing, the main scope of this task was to handle solana docker tests which look ok: https://github.com/KomodoPlatform/komodo-defi-framework/actions/runs/9991028730/job/27619183380?pr=2168 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! All solana tests should use dockarized tests and transfers / balance tests should work with it as well!
|
||
const SOLANA_CLIENT_URL: &str = "http://localhost:8899"; | ||
|
||
#[ignore] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this should be ignored, one of the purposes of having this dockerized node is to fill addresses with balances for tests. We should be able to run swap tests as these
fn solana_coin_send_and_refund_maker_payment() { |
fn solana_coin_send_and_spend_maker_payment() { |
Implement Solana Dockerized tests similar to the existing Ethereum and QTUM Dockerized tests.
And I provided the below on DM in the past
For docker tests, here are some code snippets
komodo-defi-framework/mm2src/mm2_main/tests/docker_tests_main.rs
Lines 36 to 61 in af57160
// AP: custom test runner is intended to initialize the required environment (e.g. coin daemons in the docker containers) // and then gracefully clear it by dropping the RAII docker container handlers // I've tried to use static for such singleton initialization but it turned out that despite // rustc allows to use Drop as static the drop fn won't ever be called // NB: https://github.com/rust-lang/rfcs/issues/1111 // the only preparation step required is Zcash params files downloading: // Windows - https://github.com/KomodoPlatform/komodo/blob/master/zcutil/fetch-params.bat // Linux and MacOS - https://github.com/KomodoPlatform/komodo/blob/master/zcutil/fetch-params.sh pub fn docker_tests_runner(tests: &[&TestDescAndFn]) { // pretty_env_logger::try_init(); let docker = Cli::default(); let mut containers = vec![]; // skip Docker containers initialization if we are intended to run test_mm_start only if std::env::var("_MM2_TEST_CONF").is_err() { pull_docker_image(UTXO_ASSET_DOCKER_IMAGE_WITH_TAG); pull_docker_image(QTUM_REGTEST_DOCKER_IMAGE_WITH_TAG); pull_docker_image(GETH_DOCKER_IMAGE_WITH_TAG); remove_docker_containers(UTXO_ASSET_DOCKER_IMAGE_WITH_TAG); remove_docker_containers(QTUM_REGTEST_DOCKER_IMAGE_WITH_TAG); remove_docker_containers(GETH_DOCKER_IMAGE_WITH_TAG); let utxo_node = utxo_asset_docker_node(&docker, "MYCOIN", 7000); let utxo_node1 = utxo_asset_docker_node(&docker, "MYCOIN1", 8000); let qtum_node = qtum_docker_node(&docker, 9000); let for_slp_node = utxo_asset_docker_node(&docker, "FORSLP", 10000); let geth_node = geth_docker_node(&docker, "ETH", 8545);
komodo-defi-framework/mm2src/mm2_main/tests/docker_tests/docker_tests_common.rs
Lines 338 to 348 in fa71adb
pub fn geth_docker_node<'a>(docker: &'a Cli, ticker: &'static str, port: u16) -> DockerNode<'a> { let image = GenericImage::new(GETH_DOCKER_IMAGE, "stable"); let args = vec!["--dev".into(), "--http".into(), "--http.addr=0.0.0.0".into()]; let image = RunnableImage::from((image, args)).with_mapped_port((port, port)); let container = docker.run(image); DockerNode { container, ticker: ticker.into(), port, } }
After adding solana devnet test runner as it was done for geth above, you need to specify the URL
pub static GETH_RPC_URL: &str = "http://127.0.0.1:8545";
Then you can implement functions to fill an address with coins needed for testingkomodo-defi-framework/mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs
Lines 78 to 110 in fa71adb
pub fn fill_eth(to_addr: Address, amount: U256) { let _guard = GETH_NONCE_LOCK.lock().unwrap(); let tx_request = TransactionRequest { from: geth_account(), to: Some(to_addr), gas: None, gas_price: None, value: Some(amount), data: None, nonce: None, condition: None, transaction_type: None, access_list: None, max_fee_per_gas: None, max_priority_fee_per_gas: None, }; let tx_hash = block_on(GETH_WEB3.eth().send_transaction(tx_request)).unwrap(); wait_for_confirmation(tx_hash); } fn fill_erc20(to_addr: Address, amount: U256) { let _guard = GETH_NONCE_LOCK.lock().unwrap(); let erc20_contract = Contract::from_json(GETH_WEB3.eth(), erc20_contract(), ERC20_ABI.as_bytes()).unwrap(); let tx_hash = block_on(erc20_contract.call( "transfer", (Token::Address(to_addr), Token::Uint(amount)), geth_account(), Options::default(), )) .unwrap(); wait_for_confirmation(tx_hash); }
Then you can fill the activated address after activating the coin as suchkomodo-defi-framework/mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs
Lines 265 to 299 in fa71adb
/// Creates ERC20 protocol coin supplied with 1 ETH and 100 token pub fn erc20_coin_with_random_privkey(swap_contract_address: Address) -> EthCoin { let erc20_conf = erc20_dev_conf(&erc20_contract_checksum()); let req = json!({ "method": "enable", "coin": "ERC20DEV", "swap_contract_address": swap_contract_address, "urls": [GETH_RPC_URL], }); let erc20_coin = block_on(eth_coin_from_conf_and_request( &MM_CTX, "ERC20DEV", &erc20_conf, &req, CoinProtocol::ERC20 { platform: "ETH".to_string(), contract_address: checksum_address(&format!("{:02x}", erc20_contract())), }, PrivKeyBuildPolicy::IguanaPrivKey(random_secp256k1_secret()), )) .unwrap(); let my_address = match erc20_coin.derivation_method() { DerivationMethod::SingleAddress(addr) => *addr, _ => panic!("Expected single address"), }; // 1 ETH fill_eth(my_address, U256::from(10).pow(U256::from(18))); // 100 tokens (it has 8 decimals) fill_erc20(my_address, U256::from(10000000000u64)); erc20_coin }
And this last function can be used in any test to activate a coin that has funds in it's address
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So that ignore was probably some leftover from my previous commits. If I remember correctly that test was failing for some reason and to not block this task I've disabled it for now.
As for the fill_eth
methods, last time we've discussed to try dump account states and programs into the docker container. This way we wont have to handle separate logic related to the Solana as they work differently and it is not that similar to the typical EVM contracts (It can be found here: .docker/Dockerfile.solana-tests). This way we can reduce work needed to handle those tests.
Is it ok for you to do it like that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And unfortunately it's impossible for me right now to determine if my work is working as expected as the tests are not really executing: https://github.com/KomodoPlatform/komodo-defi-framework/actions/runs/10110629981/job/27960913639?pr=2168
thread 'main' panicked at 'Reached max attempts for <<{docker:?}>>.', mm2src/mm2_main/tests/docker_tests/docker_tests_common.rs:1155:13
The result was the same (failing) with and without #[ignore] that I had on one of my test - its not related to the solana.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As for the fill_eth methods, last time we've discussed to try dump account states and programs into the docker container. This way we wont have to handle separate logic related to the Solana as they work differently and it is not that similar to the typical EVM contracts (It can be found here: .docker/Dockerfile.solana-tests). This way we can reduce work needed to handle those tests.
Is it ok for you to do it like that?
It's fine as long as we have accounts/addresses with funds to test with. solana_coin_send_and_refund_maker_payment
and solana_coin_send_and_spend_maker_payment
tests among others can't be moved to docker tests now since there are no way to have funds for these tests.
It can be found here: .docker/Dockerfile.solana-tests
This deploys the swap contract only for now. Sure we can deploy token program the same way, but we also need funds on at least one address to test with, but we will probably need more than one address since tests are run in parallel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And unfortunately it's impossible for me right now to determine if my work is working as expected as the tests are not really executing: https://github.com/KomodoPlatform/komodo-defi-framework/actions/runs/10110629981/job/27960913639?pr=2168
thread 'main' panicked at 'Reached max attempts for <<{docker:?}>>.', >mm2src/mm2_main/tests/docker_tests/docker_tests_common.rs:1155:13
The result was the same (failing) with and without #[ignore] that I had on one of my test - its not related to the solana.
Ignored the tendermint tests for now in 4277284, will revert this commit once we resolve this issue from our side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the record, I am currently attempting to adjust the scripts according to the suggestions provided. However, the most difficult part seems to be even starting the tests as they currently are. I am investigating the issues with the tests. Even though the Geth container started correctly and is accessible on port 7000, I am getting a spam of logs like the following:
30 07:04:29, docker_tests_common:167] rpc_clients:854] Transport(JsonRpcError { client_info: "coin: MYCOIN", request: JsonRpcRequest { jsonrpc: "1.0", id: "12", method: "getblockcount", params: [] }, error: Transport("Transport 'http://127.0.0.1:7000/' error: connection closed before message completed") })
It's not going too smoothly.
There are some failing tests due to depending on an external connection / url for moralis for the NFT feature, they can be ignored. That's one of the reasons we are trying to move to local test nodes for all implementations, to have a stable CI for devs. |
bde54f0
to
6c948d7
Compare
@laruh thats correct, lint check is fixed now end conflict too |
6c948d7
to
ca8a1f6
Compare
Add Dockerfile to run local solana validator for tests
unignore test test_solana_and_spl_balance_enable_spl_v2
ca8a1f6
to
47861f8
Compare
Superseded by #2187 |
Add and fix Solana docker tests
Add Dockerfile to run local solana validator for tests