test: end-to-end tests for `sendTransaction` by lpahlavi · Pull Request #104 · dfinity/sol-rpc-canister

lpahlavi · 2025-05-13T10:14:17Z

(XC-339) Add end-to-end tests for sendTransaction including fetching the slot and blockhash, building the transaction, sending it, and checking its status. The tests are done on Solana Devnet and use ic-agent to interact with the SOL RPC canister.

ninegua · 2025-05-15T10:39:41Z

integration_tests/tests/end_to_end.rs

+    let setup = Setup::new();
+
+    fn load_keypair(key: &str) -> Keypair {
+        fn try_load_keypair(key: &str) -> Result<Keypair, String> {


nit: this inner function seems only here to propagate errors. I think you can just change the test function itself to Result type and use try_load_keypair(...)? directly in the test body.

Interesting suggestion! I forgot Rust tests are allowed to return a value. However, I think I prefer to leave it as is because the original error message is quite unhelpful and this way it is clear what the problem is if this ever happens again. WDYT?

gregorydemay

Thanks for this PR @lpahlavi ! Nice to see some transaction working end-to-end!

integration_tests/src/wallet.rs

gregorydemay · 2025-05-15T13:17:36Z

integration_tests/src/wallet.rs

@@ -0,0 +1,72 @@
+//! Module to interact with a [cycles wallet](https://github.com/dfinity/cycles-wallet) canister.


I think this refactoring makes sense, but I would have done it in a separate PR. As far as I can tell it does not have anything to do with the purpose of this PR.

Yes you are right. I did have to refactor the code in this module to some extent to work with the new ic-agent runtime but the changes were minimal.

integration_tests/src/lib.rs

integration_tests/tests/end_to_end.rs

gregorydemay · 2025-05-15T13:39:28Z

integration_tests/tests/end_to_end.rs

+    let blockhash = Hash::from_str(&block.blockhash).expect("Failed to parse blockhash");
+
+    let transaction_amount = 1_000;
+    let transaction = Transaction::new_signed_with_payer(


I think it would be useful to also call getRecentPrioritizationFees and use this to set a priority fee+compute unit limit (like in here. Based on the transaction you mentioned, 150 CUL should be ok). This is important because probably needed on mainnet, and since this adds again some latency, we need to make sure that everything works together.

Good idea! I added your suggestion. Question: I used the 'max' value from the recent prioritization fees. We could also used e.g. the mean, median, etc. Any preferences? On Devnet this shouldn't really make a difference but we might want to move this test to Mainnet at some point.

I used the 'max' value from the recent prioritization fees. We could also used e.g. the mean, median, etc. Any preferences?

I think for now this is fine. There is probably a library somewhere to handle this type of calculation. For Mainnet the max is for sure not recommended, there I would rather use something like median.

I didn't see any library for this with a quick search, but I did change it to use the median which should be a bit more reasonable but still robust. We can always revisit this later if we have problems when trying to run the tests on Mainnet.

integration_tests/tests/end_to_end.rs

lpahlavi

Thanks a lot for the review feedback @ninegua and @gregorydemay! I've addressed all of your comments. The new diff is on the larger side since I moved the test to a separate crate, however the changes are not as big as it seems.

lpahlavi · 2025-05-16T14:26:30Z

integration_tests/src/wallet.rs

@@ -0,0 +1,72 @@
+//! Module to interact with a [cycles wallet](https://github.com/dfinity/cycles-wallet) canister.


Yes you are right. I did have to refactor the code in this module to some extent to work with the new ic-agent runtime but the changes were minimal.

integration_tests/src/wallet.rs

integration_tests/tests/end_to_end.rs

lpahlavi · 2025-05-16T14:29:21Z

integration_tests/tests/end_to_end.rs

+    let blockhash = Hash::from_str(&block.blockhash).expect("Failed to parse blockhash");
+
+    let transaction_amount = 1_000;
+    let transaction = Transaction::new_signed_with_payer(


Good idea! I added your suggestion. Question: I used the 'max' value from the recent prioritization fees. We could also used e.g. the mean, median, etc. Any preferences? On Devnet this shouldn't really make a difference but we might want to move this test to Mainnet at some point.

integration_tests/tests/end_to_end.rs

lpahlavi · 2025-05-16T14:45:36Z

integration_tests/tests/end_to_end.rs

+    let setup = Setup::new();
+
+    fn load_keypair(key: &str) -> Keypair {
+        fn try_load_keypair(key: &str) -> Result<Keypair, String> {


Interesting suggestion! I forgot Rust tests are allowed to return a value. However, I think I prefer to leave it as is because the original error message is quite unhelpful and this way it is clear what the problem is if this ever happens again. WDYT?

Stale

gregorydemay

Thanks @lpahlavi for those e2e tests! having them in a dedicated crate definitely looks cleaner!

end_to_end_tests/Cargo.toml

end_to_end_tests/tests/end_to_end.rs

gregorydemay · 2025-05-16T18:52:02Z

end_to_end_tests/tests/end_to_end.rs

+    // TODO XC-339: Use reasonable value here
+    let set_cu_limit_ix = ComputeBudgetInstruction::set_compute_unit_limit(1000);


nit: is that something you want to do in a follow-up PR?

Actually I just wanted to wait for the tests to go through once (hence the high temporary value) to get an idea of a reasonable value. I had a look at the transaction that went through which had a CU count of 450, and just added a small buffer so it should almost always go through (since there can be small variations in CU count).

gregorydemay · 2025-05-16T18:59:18Z

integration_tests/tests/end_to_end.rs

+    let blockhash = Hash::from_str(&block.blockhash).expect("Failed to parse blockhash");
+
+    let transaction_amount = 1_000;
+    let transaction = Transaction::new_signed_with_payer(


I used the 'max' value from the recent prioritization fees. We could also used e.g. the mean, median, etc. Any preferences?

I think for now this is fine. There is probably a library somewhere to handle this type of calculation. For Mainnet the max is for sure not recommended, there I would rather use something like median.

integration_tests/tests/end_to_end.rs

## 🤖 New release * `sol_rpc_canister`: 0.2.0 <details><summary>Changelog</summary> <blockquote> ## [0.2.0] - 2025-05-27 ### Added - Add client method to sign a transaction ([#113](#113)) - Add Chainstack RPC provider ([#118](#118)) - Add support for `getSignaturesForAddress` ([#106](#106)) - Add support for `getSignatureStatuses` RPC method ([#96](#96)) - Add support for `getTokenAccountBalance` RPC method ([#90](#90)) - Add support for `getTransaction` RPC method ([#68](#68)) - Add `getBlock` RPC method ([#53](#53)) - Add `sendTransaction` RPC method ([#59](#59)) - Add NOTICE to Apache license ([#60](#60)) - Add `getAccountInfo` RPC method ([#49](#49)) - Add metrics ([#41](#41)) - Add logging crate ([#13](#13)) - Add support for override providers ([#12](#12)) ### Changed - Bump version to 0.2 - Candid NonZeroU8 ([#108](#108)) - Add `RoundingError` to `sol_rpc_types` ([#105](#105)) - Use secure primitive types for `Pubkey`, `Signature` and `Hash` ([#98](#98)) - Add support for `getRecentPrioritizationFees` ([#92](#92)) - Simplify API keys provisioning script ([#89](#89)) - Release v0.1.0 ([#88](#88)) - Use `canlog_derive` and `canlog` from crates.io ([#84](#84)) - Release pipeline ([#4](#4)) - Clean-up TODOs ([#81](#81)) - Support method `getBalance` ([#74](#74)) - Remove http_types module and use external ic-http-types crate ([#73](#73)) - Rename some enum variants to camel case when serializing ([#72](#72)) - Use constant size JSON-RPC request ID ([#62](#62)) - Use method from JSON-RPC request for metric ([#61](#61)) - Cycles cost ([#52](#52)) - Client builder ([#54](#54)) - Round result from `getSlot` RPC method ([#48](#48)) - Use `canhttp` `multi` feature ([#46](#46)) - Implement a method for making generic RPC request ([#39](#39)) - Implement getSlot RPC method ([#33](#33)) - Add some tested RPC providers for Solana Mainnet and Devnet ([#15](#15)) - Streamline providers ([#32](#32)) - Update rust toolchain to 1.85 ([#21](#21)) - Remove unnecessary Storable implementations ([#14](#14)) - Add support for API keys ([#10](#10)) - Hard-code SOL RPC providers ([#9](#9)) - Reproducible build ([#3](#3)) - Initial cargo workspace and build pipeline ([#2](#2)) ### Fixed - Missing `TraceHttp` logs ([#129](#129)) - End-to-end tests for `sendTransaction` ([#104](#104)) - Unit test for `getRecentPrioritizationFees` parameters serialization ([#107](#107)) - Integration test for `verifyApiKey` ([#82](#82)) - Set `maxSupportedTransactionVersion` to zero for end-to-end tests ([#85](#85)) - API keys ([#58](#58)) - End-to-end tests ([#45](#45)) - Correct Solana cluster for dRPC and Helius providers ([#47](#47)) - E2e test with Solana test validator ([#20](#20)) - Create test canister on ICP mainnet ([#8](#8)) ### Removed - Remove default/non-default providers ([#122](#122)) </blockquote> </details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/). --------- Co-authored-by: gregorydemay <gregory.demay@dfinity.org> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

## 🤖 New release * `sol_rpc_canister`: 1.0.0 <details><summary>Changelog</summary> <blockquote> ## [1.0.0] - 2025-06-12 ### Added - Add support for `transactionDetails=accounts` ([#139](#139)) - Add support for `rewards` parameter for `getBlock` ([#135](#135)) - Add helper methods for request builders ([#136](#136)) - Add client method to sign a transaction ([#113](#113)) - Add Chainstack RPC provider ([#118](#118)) - Add support for `getSignaturesForAddress` ([#106](#106)) - Add support for `getSignatureStatuses` RPC method ([#96](#96)) - Add support for `getTokenAccountBalance` RPC method ([#90](#90)) - Add support for `getTransaction` RPC method ([#68](#68)) - Add `getBlock` RPC method ([#53](#53)) - Add `sendTransaction` RPC method ([#59](#59)) - Add NOTICE to Apache license ([#60](#60)) - Add `getAccountInfo` RPC method ([#49](#49)) - Add metrics ([#41](#41)) - Add logging crate ([#13](#13)) - Add support for override providers ([#12](#12)) ### Changed - Bump to 1.0.0 - Inline request parameters `is_default_config` methods ([#138](#138)) - Release v0.2.0 ([#131](#131)) - Bump version and use a release notes template ([#130](#130)) - Candid NonZeroU8 ([#108](#108)) - Add `RoundingError` to `sol_rpc_types` ([#105](#105)) - Use secure primitive types for `Pubkey`, `Signature` and `Hash` ([#98](#98)) - Add support for `getRecentPrioritizationFees` ([#92](#92)) - Simplify API keys provisioning script ([#89](#89)) - Release v0.1.0 ([#88](#88)) - Use `canlog_derive` and `canlog` from crates.io ([#84](#84)) - Release pipeline ([#4](#4)) - Clean-up TODOs ([#81](#81)) - Support method `getBalance` ([#74](#74)) - Remove http_types module and use external ic-http-types crate ([#73](#73)) - Rename some enum variants to camel case when serializing ([#72](#72)) - Use constant size JSON-RPC request ID ([#62](#62)) - Use method from JSON-RPC request for metric ([#61](#61)) - Cycles cost ([#52](#52)) - Client builder ([#54](#54)) - Round result from `getSlot` RPC method ([#48](#48)) - Use `canhttp` `multi` feature ([#46](#46)) - Implement a method for making generic RPC request ([#39](#39)) - Implement getSlot RPC method ([#33](#33)) - Add some tested RPC providers for Solana Mainnet and Devnet ([#15](#15)) - Streamline providers ([#32](#32)) - Update rust toolchain to 1.85 ([#21](#21)) - Remove unnecessary Storable implementations ([#14](#14)) - Add support for API keys ([#10](#10)) - Hard-code SOL RPC providers ([#9](#9)) - Reproducible build ([#3](#3)) - Initial cargo workspace and build pipeline ([#2](#2)) ### Fixed - Missing `TraceHttp` logs ([#129](#129)) - End-to-end tests for `sendTransaction` ([#104](#104)) - Unit test for `getRecentPrioritizationFees` parameters serialization ([#107](#107)) - Integration test for `verifyApiKey` ([#82](#82)) - Set `maxSupportedTransactionVersion` to zero for end-to-end tests ([#85](#85)) - API keys ([#58](#58)) - End-to-end tests ([#45](#45)) - Correct Solana cluster for dRPC and Helius providers ([#47](#47)) - E2e test with Solana test validator ([#20](#20)) - Create test canister on ICP mainnet ([#8](#8)) ### Removed - Remove default/non-default providers ([#122](#122)) </blockquote> </details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/). --------- Co-authored-by: gregorydemay <gregory.demay@dfinity.org> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: gregorydemay <112856886+gregorydemay@users.noreply.github.com>

lpahlavi force-pushed the lpahlavi/XC-339-send-transaction-end-to-end-tests-v2 branch from 36e70b1 to bc06eed Compare May 13, 2025 10:18

lpahlavi marked this pull request as ready for review May 13, 2025 14:29

lpahlavi requested a review from a team as a code owner May 13, 2025 14:29

lpahlavi changed the base branch from main to lpahlavi/XC-291-get-signature-statuses May 13, 2025 15:09

Base automatically changed from lpahlavi/XC-291-get-signature-statuses to main May 14, 2025 17:14

lpahlavi added 15 commits May 15, 2025 07:46

XC-339: Add end-to-end tests for sendTransaction

0ffba32

XC-339: Update CI pipeline

fdf3179

XC-339: Update CI pipeline

771b200

XC-339: Update Github secret names

ea9ec15

XC-339: Use Secp256k1Identity

a0272af

XC-339: Clippy

7809c8c

XC-339: Add default IC gateway

516df4c

XC-339: Fix keypair parsing

e15b2f0

XC-339: Clippy

91b7c12

XC-339: Deserialize as vector

0da271e

XC-339: Don't use dRPC for devnet

4fe9d6e

XC-339: Clippy

a4baa7a

XC-339: Use 2 out of 3 threshold consensus

840bfc3

XC-339: Clippy

6a88da5

XC-339: Update providers for end-to-end tests

dee1b55

lpahlavi force-pushed the lpahlavi/XC-339-send-transaction-end-to-end-tests-v2 branch from ceb0c9c to dee1b55 Compare May 15, 2025 05:46

lpahlavi requested review from gregorydemay and ninegua May 15, 2025 05:48

XC-339: Fix client call

7c27f2c

ninegua reviewed May 15, 2025

View reviewed changes

ninegua previously approved these changes May 15, 2025

View reviewed changes

gregorydemay reviewed May 15, 2025

View reviewed changes

lpahlavi added 2 commits May 16, 2025 16:23

XC-339: Move end-to-end tests to separate crate

f631bdd

XC-339: Address review feedback

01a8b07

lpahlavi commented May 16, 2025

View reviewed changes

XC-339: Exlude E2E tests from unit tests in CI pipeline

bc8da9a

lpahlavi requested a review from ninegua May 16, 2025 14:53

lpahlavi added 2 commits May 16, 2025 17:08

XC-339: Remove unused dependencies

377ee76

XC-339: Update CU limit

08d576b

gregorydemay approved these changes May 16, 2025

View reviewed changes

lpahlavi added 2 commits May 19, 2025 08:10

XC-339: Address review feedback

e09e970

XC-339: Add import for decode_cycles_wallet_response

275c873

lpahlavi merged commit ca315c9 into main May 19, 2025
12 checks passed

lpahlavi deleted the lpahlavi/XC-339-send-transaction-end-to-end-tests-v2 branch May 19, 2025 07:44

github-actions bot mentioned this pull request May 27, 2025

chore: release v0.2.0 #131

Merged

github-actions bot mentioned this pull request Jun 12, 2025

chore: release v1.0.0 #146

Merged

github-actions bot mentioned this pull request Jul 31, 2025

chore: release #189

Closed

		@@ -0,0 +1,72 @@
		//! Module to interact with a [cycles wallet](https://github.com/dfinity/cycles-wallet) canister.

		// TODO XC-339: Use reasonable value here
		let set_cu_limit_ix = ComputeBudgetInstruction::set_compute_unit_limit(1000);

Comments

Conversation

lpahlavi commented May 13, 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!

Choose a reason for hiding this comment

Uh oh!

gregorydemay left a comment

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!

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lpahlavi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gregorydemay left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lpahlavi May 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lpahlavi commented May 13, 2025 •

edited

Loading

lpahlavi May 19, 2025 •

edited

Loading