-
Notifications
You must be signed in to change notification settings - Fork 50
Stranzhay/upgrade to solana 2 #236
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
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis update synchronizes dependency and environment versions across JavaScript and Rust clients, program code, and CI workflows. It upgrades Rust, Solana, and related libraries, modifies test concurrency to run serially, adjusts re-exports and trait constants in Rust, and adds Protobuf compiler installation steps in CI workflows. No new features or exported API changes are introduced. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant CI Workflow
participant System Dependencies
Developer->>CI Workflow: Push code/update dependencies
CI Workflow->>System Dependencies: Install Rust 1.83.0, Solana 2.2.1, Protobuf compiler
CI Workflow->>CI Workflow: Build, lint, test code
CI Workflow->>Developer: Report build/test results
sequenceDiagram
participant TestRunner
participant LocalValidator
participant Asset/Collection Creation
TestRunner->>TestRunner: Run tests serially (test.serial)
loop For each asset/collection
TestRunner->>Asset/Collection Creation: await create()
Asset/Collection Creation-->>LocalValidator: Submit transaction
LocalValidator-->>Asset/Collection Creation: Confirm creation
end
TestRunner->>TestRunner: Assert test results
Possibly related PRs
Suggested reviewers
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
Cargo.lock
is excluded by!**/*.lock
clients/js/pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
clients/js/package.json
(1 hunks)clients/rust/Cargo.toml
(1 hunks)clients/rust/src/hooked/mod.rs
(0 hunks)programs/mpl-core/Cargo.toml
(1 hunks)
💤 Files with no reviewable changes (1)
- clients/rust/src/hooked/mod.rs
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Programs / Test (mpl-core)
- GitHub Check: Programs / Build
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
🔇 Additional comments (7)
clients/js/package.json (1)
41-41
: Verify compatibility of @solana/web3.js upgrade
Upgrading the devDependency to^1.98.2
may introduce breaking API changes. Ensure client tests pass and review any low-level Web3.js calls for required updates.clients/rust/Cargo.toml (2)
36-36
: Ensure solana-program-test v2.0 API compatibility
Bumping to"2.0"
may introduce test framework changes—run your test suite to confirm no failures.
37-37
: Validate solana-sdk v2.0 upgrade
Upgradingsolana-sdk
can contain breaking changes in client interfaces. Audit any direct SDK usage and reruncargo test
.programs/mpl-core/Cargo.toml (4)
16-16
: Verify mpl-bubblegum v2.1.0 API changes
The major bump can include breaking changes—ensure any bubblegum-related code is updated and tested.
19-19
: Align Solana program to v2.0
Switching tosolana-program = "2.0"
requires following the v2 migration guide (updated macros, types). Confirm macros likedeclare_id!
still work as expected.
22-22
: Validate mpl-utils v0.4.0 behavior
Ensure that utility functions haven’t changed in ways that break your program logic; run integration tests.
23-23
: Confirm spl-noop v1.0.0 CPI feature
Upgrading tospl-noop = "1.0.0"
with thecpi
feature may alter its semantics—run CPI tests to validate.
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.
Actionable comments posted: 2
♻️ Duplicate comments (1)
clients/rust/Cargo.toml (1)
29-29
: Consider locking to a patch version.
Specifying"2.0.0"
instead of"2.0"
prevents unintended minor or patch upgrades.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
📒 Files selected for processing (3)
clients/js/package.json
(2 hunks)clients/rust/Cargo.toml
(2 hunks)programs/mpl-core/Cargo.toml
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Programs / Test (mpl-core)
- GitHub Check: Programs / Build
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
clients/js/package.json (2)
3-3
: Approve version bump to 1.5.0.
Version upgrade aligns with the coordinated release plan and changelog.
41-41
: Verify compatibility of @solana/web3.js update.
Run the full test and integration suite to ensure no breaking changes with^1.98.2
.clients/rust/Cargo.toml (1)
8-8
: Approve crate version bump to 0.11.0.
This matches the broadermpl-core
version synchronization.programs/mpl-core/Cargo.toml (2)
3-3
: Approve program crate version bump to 0.2.0.
The new version aligns with the public API changes inmpl-core-program
.
16-23
: Verify breaking changes in bumped dependencies.
Major bumps formpl-bubblegum
,mpl-utils
, andspl-noop
may introduce API changes. Runcargo test
and update code/tests accordingly.
clients/rust/Cargo.toml
Outdated
solana-program-test = "2.0" | ||
solana-sdk = "2.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.
🧹 Nitpick (assertive)
Specify full patch versions for dev-dependencies.
Lock solana-program-test
and solana-sdk
to "2.0.0"
to ensure deterministic builds.
-solana-program-test = "2.0"
-solana-sdk = "2.0"
+solana-program-test = "2.0.0"
+solana-sdk = "2.0.0"
🤖 Prompt for AI Agents
In clients/rust/Cargo.toml at lines 36 to 37, the versions for dev-dependencies
solana-program-test and solana-sdk are specified as "2.0". Change these to the
full patch version "2.0.0" to ensure deterministic builds by locking the
dependencies to exact versions.
programs/mpl-core/Cargo.toml
Outdated
num-derive = "^0.3" | ||
num-traits = "^0.2" | ||
solana-program = "^1.17" | ||
solana-program = "2.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.
🧹 Nitpick (assertive)
Consider locking solana-program
to a patch version.
Pin to "2.0.0"
to avoid unintended updates within the 2.x series.
-solana-program = "2.0"
+solana-program = "2.0.0"
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
solana-program = "2.0" | |
-solana-program = "2.0" | |
+solana-program = "2.0.0" |
🤖 Prompt for AI Agents
In programs/mpl-core/Cargo.toml at line 19, the dependency version for
solana-program is set to "2.0", which allows updates to any 2.x patch version.
Change this to "2.0.0" to lock the dependency to that specific patch version and
prevent unintended updates within the 2.x series.
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lock
is excluded by!**/*.lock
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (1)
.github/.env
(1 hunks)
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.github/.env
[warning] 6-6: [UnorderedKey] The COMMIT_USER_NAME key should go before the NODE_VERSION key
[warning] 7-7: [UnorderedKey] The COMMIT_USER_EMAIL key should go before the COMMIT_USER_NAME key
[warning] 8-8: [EndingBlankLine] No blank line at the end of the file
[warning] 8-8: [UnorderedKey] The DEPLOY_SOLANA_VERSION key should go before the NODE_VERSION key
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
🔇 Additional comments (2)
.github/.env (2)
4-4
: Bump Rust toolchain version
TheRUST_VERSION
is updated to 1.84.0—ensure that yourrust-toolchain
(or CI matrix) is aligned to this version to avoid mismatches.#!/bin/bash # Verify rust-toolchain alignment if [ -f rust-toolchain ]; then rg -n '1\.84\.0' rust-toolchain || echo "⚠️ rust-toolchain not set to 1.84.0" else echo "ℹ️ No rust-toolchain file found; consider adding one to enforce RUST_VERSION" fi
5-5
: Upgrade Solana version across clients
TheSOLANA_VERSION
is now 2.0.26—confirm that all Solana deps in your Rust & JS clients match this bump.#!/bin/bash # Verify Solana version consistency files=(clients/rust/Cargo.toml programs/mpl-core/Cargo.toml clients/js/package.json) for f in "${files[@]}"; do if ! rg -q '2\.0\.26' "$f"; then echo "⚠️ Missing Solana v2.0.26 in $f" fi done
.github/.env
Outdated
COMMIT_USER_NAME=github-actions | ||
[email protected] | ||
DEPLOY_SOLANA_VERSION=1.18.19 | ||
DEPLOY_SOLANA_VERSION=2.0.26 |
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.
🧹 Nitpick (assertive)
Add trailing newline & group version keys
Consider adding a blank line at EOF to satisfy dotenv-linter and move DEPLOY_SOLANA_VERSION
next to the other version entries (RUST_VERSION
, SOLANA_VERSION
) for better grouping.
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 8-8: [EndingBlankLine] No blank line at the end of the file
[warning] 8-8: [UnorderedKey] The DEPLOY_SOLANA_VERSION key should go before the NODE_VERSION key
🤖 Prompt for AI Agents
In .github/.env at line 8, add a blank newline at the end of the file to satisfy
dotenv-linter requirements and move the DEPLOY_SOLANA_VERSION key next to the
other version keys like RUST_VERSION and SOLANA_VERSION to group all
version-related entries together for better organization.
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.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/.env (1)
5-8
: Group version keys together
For better organization, move theDEPLOY_SOLANA_VERSION
entry next to the other version variables (RUST_VERSION
andSOLANA_VERSION
).Proposed diff:
RUST_VERSION=1.79.0 SOLANA_VERSION=2.0.17 -COMMIT_USER_NAME=github-actions -[email protected] DEPLOY_SOLANA_VERSION=2.0.17 +COMMIT_USER_NAME=github-actions +[email protected]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
.github/.env
(1 hunks)
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.github/.env
[warning] 6-6: [UnorderedKey] The COMMIT_USER_NAME key should go before the NODE_VERSION key
[warning] 7-7: [UnorderedKey] The COMMIT_USER_EMAIL key should go before the COMMIT_USER_NAME key
[warning] 8-8: [UnorderedKey] The DEPLOY_SOLANA_VERSION key should go before the NODE_VERSION key
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Programs / Build
- GitHub Check: Programs / Test (mpl-core)
- GitHub Check: Analyze (actions)
- GitHub Check: Analyze (javascript-typescript)
.github/workflows/build-programs.yml
Outdated
name: Build | ||
runs-on: ubuntu-latest | ||
steps: | ||
- name: Git checkout | ||
uses: actions/checkout@v4 | ||
with: | ||
ref: ${{ inputs.git_ref }} | ||
|
||
- name: Load environment variables | ||
run: cat .github/.env >> $GITHUB_ENV | ||
- name: Load environment variables | ||
run: cat .github/.env >> $GITHUB_ENV | ||
|
||
- name: Install Rust | ||
uses: metaplex-foundation/actions/install-rust@v1 | ||
with: | ||
toolchain: ${{ inputs.rust || env.RUST_VERSION }} | ||
- name: Install Rust | ||
uses: metaplex-foundation/actions/install-rust@v1 | ||
with: | ||
toolchain: ${{ inputs.rust || env.RUST_VERSION }} | ||
|
||
- name: Install Solana | ||
uses: metaplex-foundation/actions/install-solana@v1 | ||
with: | ||
version: ${{ inputs.solana || env.SOLANA_VERSION }} | ||
cache: ${{ env.CACHE }} | ||
- name: Install Solana | ||
uses: metaplex-foundation/actions/install-solana@v1 | ||
with: | ||
version: ${{ inputs.solana || env.SOLANA_VERSION }} | ||
cache: ${{ env.CACHE }} | ||
|
||
- name: Cache program dependencies | ||
if: env.CACHE == 'true' | ||
uses: metaplex-foundation/actions/cache-programs@v1 | ||
- name: Cache program dependencies | ||
if: env.CACHE == 'true' | ||
uses: metaplex-foundation/actions/cache-programs@v1 | ||
|
||
- name: Build programs | ||
shell: bash | ||
working-directory: configs/scripts/program | ||
run: ./build.sh | ||
env: | ||
PROGRAMS: ${{ env.PROGRAMS }} | ||
- name: Build programs | ||
shell: bash | ||
working-directory: configs/scripts/program | ||
run: ./build.sh | ||
env: | ||
PROGRAMS: ${{ env.PROGRAMS }} | ||
|
||
- name: Sanitize Ref | ||
id: sanitize | ||
shell: bash | ||
run: | | ||
REF="${{ inputs.git_ref }}" | ||
if [ -z "$REF" ]; then | ||
REF="default" | ||
fi | ||
SANITIZED=${REF//\//-} | ||
echo "sanitized=$SANITIZED" >> "$GITHUB_OUTPUT" | ||
- name: Upload program builds | ||
uses: actions/upload-artifact@v4 | ||
with: | ||
name: program-builds-${{ steps.sanitize.outputs.sanitized }} | ||
# First wildcard ensures exported paths are consistently under the programs folder. | ||
path: ./program*/.bin/*.so | ||
include-hidden-files: true | ||
if-no-files-found: error | ||
- name: Sanitize Ref | ||
id: sanitize | ||
shell: bash | ||
run: | | ||
REF="${{ inputs.git_ref }}" | ||
if [ -z "$REF" ]; then | ||
REF="default" | ||
fi | ||
SANITIZED=${REF//\//-} | ||
echo "sanitized=$SANITIZED" >> "$GITHUB_OUTPUT" | ||
|
||
- name: Upload program builds | ||
uses: actions/upload-artifact@v4 | ||
with: | ||
name: program-builds-${{ steps.sanitize.outputs.sanitized }} | ||
# First wildcard ensures exported paths are consistently under the programs folder. | ||
path: ./program*/.bin/*.so | ||
include-hidden-files: true | ||
if-no-files-found: error |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
name: Build | ||
runs-on: ubuntu-latest | ||
steps: | ||
- name: Git checkout | ||
uses: actions/checkout@v4 | ||
with: | ||
ref: ${{ inputs.git_ref }} | ||
|
||
- name: Load environment variables | ||
run: cat .github/.env >> $GITHUB_ENV | ||
- name: Load environment variables | ||
run: cat .github/.env >> $GITHUB_ENV | ||
|
||
- name: Install Rust | ||
uses: metaplex-foundation/actions/install-rust@v1 | ||
with: | ||
toolchain: ${{ inputs.rust || env.RUST_VERSION }} | ||
- name: Install Rust | ||
uses: metaplex-foundation/actions/install-rust@v1 | ||
with: | ||
toolchain: ${{ inputs.rust || env.RUST_VERSION }} | ||
|
||
- name: Install Solana | ||
uses: metaplex-foundation/actions/install-solana@v1 | ||
with: | ||
version: ${{ inputs.solana || env.SOLANA_VERSION }} | ||
cache: ${{ env.CACHE }} | ||
- name: Install Solana | ||
uses: metaplex-foundation/actions/install-solana@v1 | ||
with: | ||
version: ${{ inputs.solana || env.SOLANA_VERSION }} | ||
cache: ${{ env.CACHE }} | ||
|
||
- name: Cache Rust client test dependencies | ||
uses: metaplex-foundation/actions/cache-crate@v1 | ||
with: | ||
folder: ./clients/rust | ||
key: rust-client-test | ||
- name: Cache Rust client test dependencies | ||
uses: metaplex-foundation/actions/cache-crate@v1 | ||
with: | ||
folder: ./clients/rust | ||
key: rust-client-test | ||
|
||
- name: Run cargo clippy | ||
uses: actions-rs/cargo@v1 | ||
with: | ||
command: clippy | ||
args: --all-targets --all-features --no-deps --manifest-path ./clients/rust/Cargo.toml | ||
- name: Run cargo clippy | ||
uses: actions-rs/cargo@v1 | ||
with: | ||
command: clippy | ||
args: --all-targets --all-features --no-deps --manifest-path ./clients/rust/Cargo.toml | ||
|
||
- name: Build Rust client | ||
shell: bash | ||
working-directory: clients/rust | ||
run: cargo build --all-features --release | ||
- name: Build Rust client | ||
shell: bash | ||
working-directory: clients/rust | ||
run: cargo build --all-features --release | ||
|
||
- name: Upload Rust client builds | ||
uses: actions/upload-artifact@v4 | ||
with: | ||
name: rust-client-builds | ||
# First wildcard ensures exported paths are consistently under the clients folder. | ||
path: ./targe*/release/*mpl_core* | ||
if-no-files-found: error | ||
- name: Upload Rust client builds | ||
uses: actions/upload-artifact@v4 | ||
with: | ||
name: rust-client-builds | ||
# First wildcard ensures exported paths are consistently under the clients folder. | ||
path: ./targe*/release/*mpl_core* | ||
if-no-files-found: error |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
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.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/build-programs.yml
(1 hunks).github/workflows/build-rust-client.yml
(1 hunks)idls/mpl_core.json
(1 hunks)
🧰 Additional context used
🪛 GitHub Check: CodeQL
.github/workflows/build-programs.yml
[warning] 30-81: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}
.github/workflows/build-rust-client.yml
[warning] 30-75: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}
🪛 actionlint (1.7.7)
.github/workflows/build-rust-client.yml
59-59: the runner of "actions-rs/cargo@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Programs / Build
- GitHub Check: Programs / Test (mpl-core)
🔇 Additional comments (4)
idls/mpl_core.json (1)
2-2
: Version bump from 0.1.0 to 0.2.0 is appropriate. This aligns with the coordinated version updates in thempl-core-program
and Rust client crates. Ensure you regenerate any IDL-derived code (e.g., TS bindings) and verify the bump is reflected in your build pipeline..github/workflows/build-rust-client.yml (2)
13-23
: Verify default versions align across configurations
The defaults here (Rust 1.79.0 and Solana 2.0.17) should be consistent with values in.github/.env
(which sets Rust to 1.84.0). Confirm that pinning to 1.79.0 is intentional and won’t cause unexpected mismatches.
26-26
: Approve:CACHE
moved underenv
TheCACHE
variable is now declared in the top-levelenv
block with correct indentation..github/workflows/build-programs.yml (1)
13-23
: Verify default versions align across configurations
Defaults for Rust (1.79.0) and Solana (2.0.17) should match.github/.env
versions (Rust 1.84.0). Confirm whether they should be updated to avoid divergence.
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.
Benchmark
Benchmark suite | Current: c215aa5 | Previous: 7022f76 | Ratio |
---|---|---|---|
CU: create a new, empty asset |
8166 Compute Units |
9812 Compute Units |
0.83 |
Space: create a new, empty asset |
91 Bytes |
91 Bytes |
1 |
CU: create a new, empty asset with empty collection |
16685 Compute Units |
21290 Compute Units |
0.78 |
Space: create a new, empty asset with empty collection |
91 Bytes |
91 Bytes |
1 |
CU: create a new asset with plugins |
27502 Compute Units |
31004 Compute Units |
0.89 |
Space: create a new asset with plugins |
194 Bytes |
194 Bytes |
1 |
CU: create a new asset with plugins and empty collection |
32723 Compute Units |
36655 Compute Units |
0.89 |
Space: create a new asset with plugins and empty collection |
194 Bytes |
194 Bytes |
1 |
CU: list an asset |
20181 Compute Units |
27406 Compute Units |
0.74 |
CU: sell an asset |
25331 Compute Units |
41771 Compute Units |
0.61 |
CU: list an asset with empty collection |
24878 Compute Units |
35561 Compute Units |
0.70 |
CU: sell an asset with empty collection |
33050 Compute Units |
55047 Compute Units |
0.60 |
CU: list an asset with collection royalties |
24063 Compute Units |
36875 Compute Units |
0.65 |
CU: sell an asset with collection royalties |
36114 Compute Units |
60830 Compute Units |
0.59 |
CU: transfer an empty asset |
3826 Compute Units |
5262 Compute Units |
0.73 |
CU: transfer an empty asset with empty collection |
5463 Compute Units |
8036 Compute Units |
0.68 |
CU: transfer an asset with plugins |
7277 Compute Units |
11483 Compute Units |
0.63 |
CU: transfer an asset with plugins and empty collection |
8914 Compute Units |
14257 Compute Units |
0.63 |
This comment was automatically generated by workflow using github-action-benchmark.
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.
Actionable comments posted: 2
♻️ Duplicate comments (5)
.github/workflows/build-rust-client.yml (3)
1-3
: Add apermissions
block to enforce least-privilege.
The workflow still runs with the default, overly-broad GITHUB_TOKEN permissions. Specify something like:permissions: contents: readto satisfy CodeQL guidance and GitHub best practices.
4-12
: Providedescription
andrequired: true
for eachworkflow_call
input.
This mirrors the validation you already added forworkflow_dispatch
and avoids silent mis-configuration when the workflow is invoked programmatically.
61-65
: Updateactions-rs/cargo
to a supported runner.
actions-rs/cargo@v1
is flagged as too old by actionlint. Pin to the latest maintenance tag, e.g.:- uses: actions-rs/cargo@v1 + uses: actions-rs/[email protected].github/workflows/build-programs.yml (2)
1-3
: Add apermissions
block to enforce least-privilege.
Limit the default GITHUB_TOKEN scope (e.g.,contents: read
) to comply with GitHub security guidance.
4-12
: Fill inworkflow_call
input metadata (description
,required
).
Ensures stronger validation parity withworkflow_dispatch
inputs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
📒 Files selected for processing (2)
.github/workflows/build-programs.yml
(1 hunks).github/workflows/build-rust-client.yml
(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/build-rust-client.yml
62-62: the runner of "actions-rs/cargo@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 GitHub Check: CodeQL
.github/workflows/build-rust-client.yml
[warning] 30-78: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}
.github/workflows/build-programs.yml
[warning] 30-84: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Programs / Build
- GitHub Check: Programs / Test (mpl-core)
- GitHub Check: Analyze (javascript-typescript)
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.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
programs/mpl-core/Cargo.toml (1)
12-24
: Mixed version-specifier styles – pick a policy and stick to it
solana-program
,mpl-bubblegum
,mpl-utils
, andspl-noop
are pinned to an exact patch while other deps (borsh
,num-*
,strum
, …) still use caret specifiers.
Mixing styles makes the resolver’s output less predictable and muddies the intent (deterministic build vs. automatic patch uptake).Decide on one policy (pin everything or use caret everywhere) and apply consistently; otherwise future bumps will be a whack-a-mole exercise.
♻️ Duplicate comments (7)
clients/rust/Cargo.toml (1)
14-15
:kaigan
is now always compiled – intentional?
Theanchor
feature only gatesanchor-lang
, whilekaigan
(optional = false) is compiled unconditionally.
This mirrors the earlier feedback that went unresolved. If the unconditional compile is not deliberate, restore the feature flag:-anchor = ["dep:anchor-lang"] +anchor = ["dep:anchor-lang", "dep:kaigan"]Also applies to: 32-32
.github/workflows/build-rust-client.yml (4)
1-3
: Add a permissions block for least-privilege tokensCode-scanning warns that the workflow lacks an explicit
permissions:
stanza.
Add:permissions: contents: readto satisfy GHAS recommendations.
4-12
: Describe & validateworkflow_call
inputsThe three inputs under
workflow_call
have onlytype
. Adddescription:
andrequired: true
to matchworkflow_dispatch
and improve caller UX.
52-54
: Harden APT install for ProtobufUse
--no-install-recommends
and clean up afterwards to keep images slim:- run: sudo apt-get update && sudo apt-get install -y protobuf-compiler + run: | + sudo apt-get update -y + sudo apt-get install -y --no-install-recommends protobuf-compiler + sudo apt-get clean + rm -rf /var/lib/apt/lists/*
61-63
:actions-rs/cargo@v1
is flagged as outdatedActionlint still warns. Pin to latest maintenance tag:
- uses: actions-rs/cargo@v1 + uses: actions-rs/[email protected].github/workflows/build-programs.yml (2)
1-3
: Missingpermissions:
– same issue as Rust-client buildAdd:
permissions: contents: read
4-12
: Provide metadata forworkflow_call
inputsMirror the
workflow_dispatch
block withdescription
andrequired: true
so downstream callers get validation errors early.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/build-programs.yml
(1 hunks).github/workflows/build-rust-client.yml
(1 hunks)clients/rust/Cargo.toml
(2 hunks)programs/mpl-core/Cargo.toml
(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
clients/rust/Cargo.toml (1)
Learnt from: danenbm
PR: metaplex-foundation/mpl-core#225
File: clients/js/test/plugins/collection/bubblegumV2.test.ts:18-20
Timestamp: 2025-05-07T18:56:19.651Z
Learning: For test files, danenbm prefers to avoid additional dependencies by keeping constants like program IDs hardcoded directly in the test file rather than importing them from shared files.
programs/mpl-core/Cargo.toml (1)
Learnt from: blockiosaurus
PR: metaplex-foundation/mpl-core#231
File: programs/mpl-core/src/instruction.rs:48-68
Timestamp: 2025-06-05T12:01:30.794Z
Learning: In the mpl-core program, new variants must always be appended to the end of the MplAssetInstruction enum to maintain on-chain ABI compatibility. Inserting variants in the middle breaks Borsh serialization ordinals and would render existing clients unable to decode instructions correctly.
🪛 actionlint (1.7.7)
.github/workflows/build-rust-client.yml
62-62: the runner of "actions-rs/cargo@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🪛 GitHub Check: CodeQL
.github/workflows/build-rust-client.yml
[warning] 30-78: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}
.github/workflows/build-programs.yml
[warning] 30-84: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{contents: read}}
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Programs / Build
- GitHub Check: Programs / Test (mpl-core)
🔇 Additional comments (1)
programs/mpl-core/Cargo.toml (1)
16-19
: Sanity-check the Solana 2.x jump for on-chain compatibilityUpgrading a program crate straight from 1.x to
solana-program = "2.2.1"
is ABI-safe in theory, but check:
#[program]
entrypoints still compile to thesbf-solana-solana
target and passcargo-build-bpf
.- Any CPI callers (e.g. Bubblegum 2.x) are also rebuilt against 2.x; otherwise the linker will complain at runtime.
If you haven’t run an integration test on a localnet with both upgraded, please do so before merging.
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
package.json
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Programs / Test (mpl-core)
- GitHub Check: Programs / Build
🔇 Additional comments (1)
package.json (1)
30-30
: Double-check lockfile & CI after bumping to[email protected]
.Updating the
packageManager
field is enough for Corepack to fetch the exact version, but the repo must also contain a lockfile produced by that version, and CI pipelines (e.g. GitHub Actions) needcorepack enable && corepack prepare [email protected]
or similar. Otherwise developers/CI may silently fall back to a different version, causing reproducibility drift.
Please verify:
pnpm-lock.yaml
regenerated with v10.13.1.- All CI jobs pin or prepare the same version.
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.
Actionable comments posted: 1
♻️ Duplicate comments (2)
package.json (1)
2-30
: Whitespace-only diff still bloats blame history – enforce formatting instead.
The entire file is re-indented again without functional changes. Please add a repo-wide formatter rule (.editorconfig
, Prettier, etc.) or revert noise-only diffs to keep history clean.clients/js/package.json (1)
38-39
: web3.js & @types/node upgrade – rerun tests and re-classify dependency if required.
Previous review still applies: ensure the new@solana/web3.js
version doesn’t break runtime code and move it fromdevDependencies
todependencies
if imported in shipped sources.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
clients/js/package.json
(3 hunks)package.json
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Programs / Test (mpl-core)
- GitHub Check: Programs / Build
🔇 Additional comments (1)
clients/js/package.json (1)
61-61
: Redundant concurrency flag removed – nice cleanup.
Keeping the concurrency setting only in the AVA config resolves the earlier DRY violation.
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 think this looks good! Thanks for all of the glass chewing. Let's merge and see if anything breaks, haha
No description provided.