feat: add client method to sign a transaction by lpahlavi · Pull Request #113 · dfinity/sol-rpc-canister

lpahlavi · 2025-05-19T13:13:30Z

(XC-317) Add a helper method in the sol_rpc_client module to signa a Solana transaction using threshold EdDSA. This method is not part of the client struct as it does not interact with the SOL RPC canister, but instead with the management canister. However, it takes a runtime argument to be re-usable in tests and prod code.

examples/basic_solana/src/ed25519.rs

examples/basic_solana/src/lib.rs

lpahlavi · 2025-05-20T08:26:51Z

libs/client/src/threshold_signatures.rs

+/// # Ok(())
+/// # }
+/// ```
+pub async fn get_pubkey<R: Runtime>(


This method is not really necessary, but I thought that it's nice to provide some utility functions to make things as easy as possible for end users. As a bonus we can re-use this method in:

the docs for sign_transaction above

basic_solana (and use the chain code to derive child keys locally -- we could potentially even add an example in the doc here on how to derive child keys locally)

the end-to-end tests (in an upcoming PR where the end-to-end tests will use threshold signing)

Another candidate here would be a helper function to derive child public keys, but this seems like stretching it a bit imo.

WDYT?

lpahlavi · 2025-05-20T08:29:28Z

libs/client/src/threshold_signatures.rs

+/// # Ok(())
+/// # }
+/// ```
+pub async fn sign_transaction<R: Runtime>(


This method is a building block for the build_transaction method which will come in an upcoming PR. For both transactions using a recent blockhash and those using a durable nonce, it is necessary to sign the transaction (using threshold signing) if we want to have a single method call, which I find would make the most sense. The alternative would be to build an unsigned transaction, and then ask the end-users to sign the transaction themselves, but I think it's nice to make it as easy as possible for them here. Especially since we anyways need this code for end-to-end tests and basic_solana.

lpahlavi · 2025-05-20T08:31:24Z

libs/client/src/threshold_signatures.rs

+    transaction: &solana_transaction::Transaction,
+    key_id: Ed25519KeyId,
+    derivation_path: Option<&DerivationPath>,
+) -> RpcResult<solana_signature::Signature> {


The reason I don't return the signed transaction here, is that this might need to be called multiple times for transactions that have multiple signatures. AFAIK we cannot currently batch threshold signature calls, so latency might be a problem here if a transaction has many signers.

gregorydemay

Thanks @lpahlavi for this PR! some first high level comments

libs/types/src/solana/signature/mod.rs

gregorydemay · 2025-05-21T12:02:37Z

libs/client/src/threshold_sig.rs

+use sol_rpc_types::{DerivationPath, Ed25519KeyId, RpcError, RpcResult};
+
+// Source: https://internetcomputer.org/docs/current/references/t-sigs-how-it-works/#fees-for-the-t-schnorr-production-key
+const SIGN_WITH_SCHNORR_FEE: u128 = 26_153_846_153;


The fee is dependent of the key being used:

https://internetcomputer.org/docs/current/references/t-sigs-how-it-works/#fees-for-the-t-schnorr-production-key

https://internetcomputer.org/docs/current/references/t-sigs-how-it-works/#fees-for-the-t-schnorr-production-key

Interesting! I initially had a look at the ic-cdk code and it seems they don't make that distinction there... I pinged the crypto channel to check which way is correct and they said it's just a simplification in the ic-cdk code and the extra cycles just get returned. Nevertheless, easy enough for us to make the distinction here. Done.

Just one follow-up question: what about the local development key? Do you know how cycles management works with local dfx deployment? Should the fee be 0 there?

Thanks for checking with crypto!

Just one follow-up question: what about the local development key? Do you know how cycles management works with local dfx deployment? Should the fee be 0 there?

I would also expect 0.

libs/client/Cargo.toml

lpahlavi

Thanks a lot for the initial review @gregorydemay! I've incorporated all of your feedback PTAL and let me know what you think.

libs/client/Cargo.toml

lpahlavi · 2025-05-22T07:46:54Z

libs/client/src/threshold_sig.rs

+use sol_rpc_types::{DerivationPath, Ed25519KeyId, RpcError, RpcResult};
+
+// Source: https://internetcomputer.org/docs/current/references/t-sigs-how-it-works/#fees-for-the-t-schnorr-production-key
+const SIGN_WITH_SCHNORR_FEE: u128 = 26_153_846_153;


Interesting! I initially had a look at the ic-cdk code and it seems they don't make that distinction there... I pinged the crypto channel to check which way is correct and they said it's just a simplification in the ic-cdk code and the extra cycles just get returned. Nevertheless, easy enough for us to make the distinction here. Done.

Just one follow-up question: what about the local development key? Do you know how cycles management works with local dfx deployment? Should the fee be 0 there?

libs/types/src/solana/signature/mod.rs

gregorydemay

Thanks @lpahlavi for the changes! Some minor comments but otherwise LGTM!

libs/client/Cargo.toml

examples/basic_solana/src/lib.rs

libs/client/src/lib.rs

gregorydemay · 2025-05-22T11:26:08Z

end_to_end_tests/Cargo.toml

 sol_rpc_int_tests = { path = "../integration_tests" }
 solana-commitment-config = { workspace = true }
 solana-hash = { workspace = true }
+solana-message = { workspace = true }


why is this change needed?

end_to_end_tests/tests/end_to_end.rs

libs/client/src/threshold_sig.rs

examples/basic_solana/src/solana_wallet.rs

…ction

lpahlavi · 2025-05-22T14:04:08Z

@gregorydemay I realized actually that the two new methods return an RpcResult which is not really correct... On the one hand it's nice to re-use the same type as RPC calls, on the other hand the error values right now are just generic RpcError::ValidationError and it would maybe make more sense to introduce a new error type for management canister calls, or even more specifically threshold signing errors. WDYT?

gregorydemay · 2025-05-22T15:11:03Z

@gregorydemay I realized actually that the two new methods return an RpcResult which is not really correct... On the one hand it's nice to re-use the same type as RPC calls, on the other hand the error values right now are just generic RpcError::ValidationError and it would maybe make more sense to introduce a new error type for management canister calls, or even more specifically threshold signing errors. WDYT?

@lpahlavi : yes I also think using RpcResult here is wrong. I think we should return the error from the IC CDK, we don't need other errors I think (e.g., we should panic if we cannot parse the signature returned from the IC as a Signature since this would indicate a bug)

## 🤖 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>

XC-317: Add client method to sign a transaction

500f760

lpahlavi commented May 19, 2025

View reviewed changes

examples/basic_solana/src/ed25519.rs Show resolved Hide resolved

lpahlavi commented May 19, 2025

View reviewed changes

examples/basic_solana/src/lib.rs Show resolved Hide resolved

lpahlavi added 7 commits May 19, 2025 15:19

XC-317: Fix doctest

814df47

XC-317: Fix basic_solana Candid and doctest

879db3b

XC-317: Remove TODO

8d2d6d0

XC-317: Fix Candid interface

b848a0a

XC-317: Response already parsed as tuple

a273b6d

XC-317: Unnecessary tuple in docs

5dca201

XC-317: Add method to fetch EdDSA public key

8abbf47

lpahlavi commented May 20, 2025

View reviewed changes

lpahlavi added 3 commits May 20, 2025 10:51

XC-317: Clippy

81f536b

XC-317: Clippy

f52b417

XC-317: Revert aggressive refactoring

fc895e4

lpahlavi marked this pull request as ready for review May 20, 2025 09:24

lpahlavi requested a review from a team as a code owner May 20, 2025 09:24

lpahlavi requested review from gregorydemay and ninegua May 20, 2025 15:37

gregorydemay reviewed May 21, 2025

View reviewed changes

lpahlavi added 5 commits May 22, 2025 09:20

XC-317: Revert aggressive refactoring from IntelliJ

ad6a892

XC-317: Remove Ed25519 and Derivation path from sol_rpc_types crate

e1d5f2d

XC-317: Revert aggressive refactoring from IntelliJ

d9d3403

XC-317: Add threshold_sig feature flag

f533b56

XC-317: Remove sol_rpc_types::signature module

68a0a51

lpahlavi commented May 22, 2025

View reviewed changes

lpahlavi requested a review from gregorydemay May 22, 2025 09:04

gregorydemay approved these changes May 22, 2025

View reviewed changes

lpahlavi added 2 commits May 22, 2025 14:37

XC-317: Rename feature flag and basic_solana::Ed25519KeyId

1aa5552

Merge branch 'main' into lpahlavi/XC-317-client-method-to-sign-transa…

f619cf4

…ction

lpahlavi added 2 commits May 22, 2025 15:57

XC-317: Various renamings from review feedback

db463f9

XC-317: Change schnorr_public_key call to update

e823351

lpahlavi added 2 commits May 22, 2025 17:19

XC-317: Don't use RpcResult for Ed25519 module methods

cf00eb9

XC-317: Format

5940dec

lpahlavi merged commit 8c27c3a into main May 22, 2025
12 checks passed

lpahlavi deleted the lpahlavi/XC-317-client-method-to-sign-transaction branch May 22, 2025 16:08

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

Conversation

lpahlavi commented May 19, 2025

Uh oh!

Uh oh!

Uh oh!

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

lpahlavi May 20, 2025

Choose a reason for hiding this comment

Uh oh!

lpahlavi May 20, 2025

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!

gregorydemay May 21, 2025

Choose a reason for hiding this comment

Uh oh!

lpahlavi May 22, 2025

Choose a reason for hiding this comment

Uh oh!

gregorydemay May 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lpahlavi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lpahlavi May 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gregorydemay left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gregorydemay May 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lpahlavi commented May 22, 2025

Uh oh!

gregorydemay commented May 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lpahlavi May 20, 2025 •

edited

Loading