refactor: standalone `basic_solana` for ICP Ninja by lpahlavi · Pull Request #249 · dfinity/sol-rpc-canister

lpahlavi · 2025-11-11T14:46:30Z

(DEFI-2462) Refactor the basic_solana example to be fully standalone, using only published crate versions. This allows the example to be imported directly into ICP Ninja without requiring forks due to breaking changes in sol_rpc_types or sol_rpc_client (as was done in #197).

The CI pipeline is updated with more robust testing for both local and ICP Ninja deployments of basic_solana via dfx, covering macOS and Linux.

gregorydemay · 2025-11-25T08:53:49Z

.github/workflows/ci.yml

-
-      - name: 'Check ninja Cargo.toml'
-        run: python3 .github/scripts/check_ninja_cargo_toml.py
+# TODO DEFI-2462: Re-enable once `sol_rpc_client` and `sol_rpc_types` crates using `ic-cdk` v0.18.6 are published


understanding question: to make it easier to review in the future. Wouldn't it be possible to let the source code of the example as is and comment out the CI part that builds the example? So that basically just a couple of lines are to be reviewed instead of currently 700 (I know it's probably just a copy from the symlink, but the problem is that the reviewer is supposed to verify the diff, which is annoying).

The problem is that then in the meantime the ICP Ninja deployment wouldn't work anymore... Not just in the CI but generally the links to deploy to ICP Ninja would link to something that doesn't compile. I completely agree that this is very annoying and honestly I'm not sure doing this circus everytime we have a breaking change is very sustainable (especially given the tendency of ic-cdk to have breaking changes).

Maybe we should consider making the examples use the latest published release altogether? We could also have a CI pipeline job that checks that we are using the latest published version of our crates to ensure the examples don't fall behind.

WDYT?

The problem is that then in the meantime the ICP Ninja deployment wouldn't work anymore... Not just in the CI but generally the links to deploy to ICP Ninja would link to something that doesn't compile.

Good point, that wouldn't be nice.

Maybe we should consider making the examples use the latest published release altogether?

yes, I think that's a good suggestion, particularly since now that we launch the API should be much more stable.

We could also have a CI pipeline job that checks that we are using the latest published version of our crates to ensure the examples don't fall behind.

I think here we should be able to just re-use dependabot, no? I think we could just tell dependabot to update sol_rpc_client and sol_rpc_types only (to limit also the amount of noise that the other updates generates). WDYT?

Sounds good! I've refactored things so that the basic_solana example is now completely standalone (i.e. has no path dependencies in Cargo.toml and depends only on published crates).

I've also made the CI job a bit more robust to test the local and ninja deployments each with both macOS and Ubuntu. I even uncovered a small mistake in examples/basic_solana/local/dfx.json in the process.

Note that the diff is huge, but it's almost entirely the new Cargo.lock for basic_solana.

I think here we should be able to just re-use dependabot, no? I think we could just tell dependabot to update sol_rpc_client and sol_rpc_types only (to limit also the amount of noise that the other updates generates). WDYT?

That's a very good idea! I'll take care of that in a separate PR but I agree that at this point dependabot has become more or less noise...

gregorydemay · 2025-11-25T13:50:22Z

.github/workflows/ci.yml

-
-      - name: 'Check ninja Cargo.toml'
-        run: python3 .github/scripts/check_ninja_cargo_toml.py
+# TODO DEFI-2462: Re-enable once `sol_rpc_client` and `sol_rpc_types` crates using `ic-cdk` v0.18.6 are published


The problem is that then in the meantime the ICP Ninja deployment wouldn't work anymore... Not just in the CI but generally the links to deploy to ICP Ninja would link to something that doesn't compile.

Good point, that wouldn't be nice.

Maybe we should consider making the examples use the latest published release altogether?

yes, I think that's a good suggestion, particularly since now that we launch the API should be much more stable.

We could also have a CI pipeline job that checks that we are using the latest published version of our crates to ensure the examples don't fall behind.

I think here we should be able to just re-use dependabot, no? I think we could just tell dependabot to update sol_rpc_client and sol_rpc_types only (to limit also the amount of noise that the other updates generates). WDYT?

…mmand

Stale

gregorydemay

Thanks @lpahlavi !

gregorydemay · 2025-11-28T07:32:02Z

examples/basic_solana/ninja/dfx.json

      "init_arg": "( record { solana_network = opt variant { Devnet }; ed25519_key_name = opt variant { MainnetTestKey1 }; sol_rpc_canister_id = opt principal \"tghme-zyaaa-aaaar-qarca-cai\" } )"
    },
    "sol_rpc": {
      "candid": "https://github.com/dfinity/sol-rpc-canister/releases/download/v1.0.0/sol_rpc_canister.did",


For another PR, but the candid file should probably be updated with the latest release.

Good catch! I'll update it in another PR, thanks for pointing it out!

gregorydemay · 2025-11-28T07:34:38Z

examples/basic_solana/ninja/dfx.json

          "ic": "tghme-zyaaa-aaaar-qarca-cai"
        }
      },
      "wasm": "https://github.com/dfinity/sol-rpc-canister/releases/download/v1.0.0/sol_rpc_canister.wasm.gz",


similar here

In #249, the `basic_solana` example was refactored to be standalone and was thus removed from the repository's top-level Cargo workspace. This PR updates `release-plz.toml` accordingly by removing the outdated override to ignore the `basic_solana` crate to fix the `Release` job (see an example run with an error [here](https://github.com/dfinity/sol-rpc-canister/actions/runs/20909693682)).

lpahlavi changed the title ~~refactor: standalone basic_solana~~ refactor: standalone basic_solana for ICP Ninja Nov 20, 2025

lpahlavi force-pushed the lpahlavi/standalone-basic-solana branch from 4c4a4a9 to 42aea94 Compare November 20, 2025 13:24

DEFI-2462: Standalone basic_solana for ICP Ninja

e59b66b

lpahlavi force-pushed the lpahlavi/standalone-basic-solana branch from 42aea94 to e59b66b Compare November 20, 2025 13:51

lpahlavi requested a review from gregorydemay November 20, 2025 14:56

lpahlavi marked this pull request as ready for review November 20, 2025 14:56

lpahlavi mentioned this pull request Nov 24, 2025

refactor: use canister runtime crates #248

Merged

gregorydemay reviewed Nov 25, 2025

View reviewed changes

lpahlavi requested a review from gregorydemay November 25, 2025 13:19

gregorydemay previously approved these changes Nov 25, 2025

View reviewed changes

lpahlavi added 16 commits November 26, 2025 10:25

DEFI-2462: Use published crates only for basic_solana

7ffa361

DEFI-2462: Use published crates only for basic_solana v2

48a4ffa

DEFI-2462: Add examples/basic_solana/Cargo.lock

282f2ad

DEFI-2462: Fix basic_solana.wasm path to local deployment

96c4e3b

DEFI-2462: Run basic_solana deployment tests on macOS and Linux

24b4870

DEFI-2462: Use perl instead of sed to have unified macOS/Linux co…

15ebb30

…mmand

DEFI-2462: Debugging print statement

4b497a5

DEFI-2462: Escape quotes in replacement pattern

790c815

DEFI-2462: Double escape quotes in replacement pattern

8d99a9a

DEFI-2462: Triple escape quotes in replacement pattern

9048922

DEFI-2462: Revert accidental commit of dfx.json

eb0d241

DEFI-2462: Remove debugging print

ec8a115

DEFI-2462: Add canister-release profile in basic_solana Cargo.toml

a4b4b4c

DEFI-2462: Use build script for ninja deployment

d814160

DEFI-2462: Run brew install llvm on macOS

834e7a5

DEFI-2462: Set working-directory for basic_solana integration tests

6cfb3b3

lpahlavi requested a review from gregorydemay November 26, 2025 10:59

lpahlavi added 2 commits November 26, 2025 12:45

DEFI-2462: Fix BASIC_SOLANA_WASM_PATH

b9e4bd5

DEFI-2462: Remove outdated script

a39f8f5

gregorydemay approved these changes Nov 28, 2025

View reviewed changes

lpahlavi merged commit 532bdcc into main Nov 28, 2025
17 checks passed

lpahlavi deleted the lpahlavi/standalone-basic-solana branch November 28, 2025 07:47

lpahlavi mentioned this pull request Jan 12, 2026

fix: remove basic_solana override from release-plz.toml #258

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: standalone `basic_solana` for ICP Ninja#249

refactor: standalone `basic_solana` for ICP Ninja#249
lpahlavi merged 19 commits intomainfrom
lpahlavi/standalone-basic-solana

lpahlavi commented Nov 11, 2025 •

edited

Loading

Uh oh!

gregorydemay Nov 25, 2025

Uh oh!

lpahlavi Nov 25, 2025

Uh oh!

gregorydemay Nov 25, 2025

Uh oh!

lpahlavi Nov 26, 2025

Uh oh!

gregorydemay Nov 25, 2025

Uh oh!

gregorydemay left a comment

Uh oh!

gregorydemay Nov 28, 2025

Uh oh!

lpahlavi Nov 28, 2025

Uh oh!

gregorydemay Nov 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

lpahlavi commented Nov 11, 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!

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!

gregorydemay 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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lpahlavi commented Nov 11, 2025 •

edited

Loading