-
Notifications
You must be signed in to change notification settings - Fork 293
[AMM-81] Bump solana-sdk dependencies to v3 #1162
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
base: main
Are you sure you want to change the base?
Conversation
| "build": "nx run-many --target build --projects", | ||
| "start": "nx run-many --target start --projects", | ||
| "test": "nx run-many --target test --projects", | ||
| "test": "nx run-many --target test --projects --exclude=@orca-so/whirlpools-sdk-integration,@orca-so/whirlpools-rust-integration", |
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.
- The
@orca-so/whirlpools-sdk-integrationtests are duplicates of@orca-so/whirlpools-sdk - The
@orca-so/whirlpools-rust-integrationtests still fail via the tests script (but build via manualcargo buildcommands), so I am proposing to temporarily ignore these tests so that root levelyarn testcommand works
| } | ||
|
|
||
| for i in 0..3 { | ||
| for (i, _) in reward_infos.iter().enumerate().take(3) { |
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.
This was a compiler warning/suggestion
wjthieme
left a comment
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.
Small comment but lgtm
|
|
||
| [dependencies] | ||
| anchor-lang = { version = ">=0.31", optional = true } | ||
| borsh = { version = "^0.10" } |
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.
Believe there might have been a reason we need borsh 0.10 (probably that's what anchor uses but not sure)
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.
yeah, i think >=v3 upgrades borsh to 1.x. when i revert to 0.10 in Cargo.toml and try to build the rust-client, i get a massive error dump ⬇️
update: it looks like they removed support for v0.10 here: anza-xyz/solana-sdk@e70cee9
✖ nx run @orca-so/whirlpools-rust-client:build
Compiling orca_whirlpools_client v5.0.1 (/Users/jacobshiohira/projects/solana/orca/whirlpools/rust-sdk/client)
warning: unused import: `programs::*`
--> src/generated/mod.rs:15:18
|
15 | pub(crate) use programs::*;
| ^^^^^^^^^^^
|
= note: `#[warn(unused_imports)]` on by default
error[E0277]: the trait bound `Pubkey: BorshSerialize` is not satisfied
--> src/generated/accounts/adaptive_fee_tier.rs:13:10
|
13 | #[derive(BorshSerialize, BorshDeserialize, Clone, Debug, Eq, PartialEq)]
| ^^^^^^^^^^^^^^ the trait `BorshSerialize` is not implemented for `Pubkey`
|
note: there are multiple different versions of crate `borsh` in the dependency graph
--> /Users/jacobshiohira/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/borsh-0.10.4/src/ser/mod.rs:44:1
|
44 | pub trait BorshSerialize {
| ^^^^^^^^^^^^^^^^^^^^^^^^ this is the required trait
|
::: src/state/tick_array.rs:1:5
|
1 | use borsh::{BorshDeserialize, BorshSerialize};
| ----- one version of crate `borsh` used here, as a direct dependency of the current crate
|
::: src/pda/fee_tier.rs:2:5
|
2 | use solana_program_error::ProgramError;
| -------------------- one version of crate `borsh` used here, as a dependency of crate `solana_program_error`
|
::: /Users/jacobshiohira/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/solana-address-1.0.0/src/lib.rs:87:1
|
87 | pub struct Address(pub(crate) [u8; 32]);
| ------------------ this type doesn't implement the required trait
|
::: /Users/jacobshiohira/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/borsh-1.5.7/src/de/mod.rs:36:1
||
|
||
| [dependencies] | ||
| solana-program = { version = "~2.2.0" } | ||
| solana-program = { version = "~3.0.0" } |
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.
Think we can fully delete the rust/integration tests cause they don't do very much now anymore
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.
fair point! are you thinking this includes all tests in rust-sdk/integration: client (anchor, serde, solana v3 + rust orca_whirlpools_client), core (ethnum, libm), and whirlpool (solana v3 and orca_whirlpools)? or just a subset?
Description
There are a variety of updates here related to the v3 dependency upgrades:
rust-sdk/whirlpool, e.g. unused varsTesting
yarn && yarn build && yarn format && yarn lint && yarn testContainerized build:
docker compose --profile build upContainerized test:
docker compose --profile test upTask
https://linear.app/orca-so/issue/AMM-81/bump-solana-deps-v2-v3-in-whirlpool-repo