magic-program & magic-program-api updated to use bincode 2 #905
magic-program & magic-program-api updated to use bincode 2 #905
Conversation
# Conflicts: # magicblock-accounts/src/scheduled_commits_processor.rs # magicblock-committor-service/src/committor_processor.rs # magicblock-committor-service/src/intent_execution_manager/intent_execution_engine.rs # magicblock-committor-service/src/tasks/task_builder.rs
…nts.rs Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
… feat/intents/v2-simplified
# Conflicts: # magicblock-accounts/src/scheduled_commits_processor.rs
📝 WalkthroughWalkthroughThis pull request upgrades the bincode serialization library from version 1.3.3 to 2.0.1 across the magicblock-magic-program-api and programs/magicblock crates. The migration replaces direct bincode serialize/deserialize calls with bincode::serde::encode_to_vec and decode_from_slice using legacy configuration to maintain backward compatibility. Related error types are updated from bincode::Error to bincode::error::EncodeError and DecodeError. Comprehensive compatibility tests are added to validate round-trip serialization and deserialization across bincode versions. Build configuration is updated with the ignore-rust-version flag to address MSRV discrepancies between bincode 2.0.1 and Solana's toolchain. Suggested reviewers
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@magicblock-magic-program-api/src/instruction.rs`:
- Around line 205-218: The compatibility helper
assert_bincode_deserialize_compatible currently discards the bytes-read from
bincode::serde::decode_from_slice which can hide trailing-byte mismatches;
update the call to capture the returned tuple (bincode2_result, bytes_read) from
decode_from_slice on bincode1_bytes, unwrap that result, then assert bytes_read
== bincode1_bytes.len() (with a clear message) before asserting bincode2_result
equals value so you ensure full buffer consumption; refer to the function
assert_bincode_deserialize_compatible and variables bincode1_bytes and
bincode2_result.
In `@programs/magicblock/src/magic_context.rs`:
- Around line 89-115: The two duplicated helpers assert_bincode_compatible and
assert_bincode_deserialize_compatible should be extracted into a shared test
utility module (e.g., tests/utils or a new crate/test-only module) and imported
from both magic_context.rs and magicblock-magic-program-api/src/instruction.rs;
move the functions (keeping their signatures and trait bounds: T: Serialize and
T: Serialize + Deserialize + PartialEq + Debug respectively), make them pub (or
pub(crate)/pub(super) as needed), update both files to remove the local copies
and call the shared functions, and add any necessary dev-dependencies or module
path imports so both test targets can access the new utility.
In `@test-integration/Makefile`:
- Around line 205-208: The Makefile rule for target $(COMMITTOR_PROGRAM_SO) has
inconsistent continuation-line indentation (the two continuation lines currently
use a single tab); update those continuation lines in that rule to use the same
double-tab indentation as the other build rules so the recipe formatting is
consistent (i.e., change the single-tab before the cargo build-sbf line and the
subsequent continuation line to double tabs in the $(COMMITTOR_PROGRAM_SO)
rule).
| /// Helper to verify bincode 2 can deserialize bincode 1 serialized data | ||
| fn assert_bincode_deserialize_compatible<T>(value: &T, name: &str) | ||
| where | ||
| T: Serialize + for<'de> Deserialize<'de> + PartialEq + std::fmt::Debug, | ||
| { | ||
| let bincode1_bytes = bincode1::serialize(value).unwrap(); | ||
| let bincode2_result: T = bincode::serde::decode_from_slice( | ||
| &bincode1_bytes, | ||
| bincode::config::legacy(), | ||
| ) | ||
| .unwrap() | ||
| .0; | ||
| assert_eq!(&bincode2_result, value, "{} round-trip mismatch", name); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Assert full buffer consumption during decode.
Line 210-216 ignore the bytes-read value, which can hide trailing-byte mismatches in compatibility tests. Adding a simple check improves confidence.
♻️ Suggested tweak
- let bincode2_result: T = bincode::serde::decode_from_slice(
- &bincode1_bytes,
- bincode::config::legacy(),
- )
- .unwrap()
- .0;
+ let (bincode2_result, bytes_read) =
+ bincode::serde::decode_from_slice(
+ &bincode1_bytes,
+ bincode::config::legacy(),
+ )
+ .unwrap();
+ assert_eq!(
+ bytes_read,
+ bincode1_bytes.len(),
+ "{} decode did not consume full buffer",
+ name
+ );📝 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.
| /// Helper to verify bincode 2 can deserialize bincode 1 serialized data | |
| fn assert_bincode_deserialize_compatible<T>(value: &T, name: &str) | |
| where | |
| T: Serialize + for<'de> Deserialize<'de> + PartialEq + std::fmt::Debug, | |
| { | |
| let bincode1_bytes = bincode1::serialize(value).unwrap(); | |
| let bincode2_result: T = bincode::serde::decode_from_slice( | |
| &bincode1_bytes, | |
| bincode::config::legacy(), | |
| ) | |
| .unwrap() | |
| .0; | |
| assert_eq!(&bincode2_result, value, "{} round-trip mismatch", name); | |
| } | |
| /// Helper to verify bincode 2 can deserialize bincode 1 serialized data | |
| fn assert_bincode_deserialize_compatible<T>(value: &T, name: &str) | |
| where | |
| T: Serialize + for<'de> Deserialize<'de> + PartialEq + std::fmt::Debug, | |
| { | |
| let bincode1_bytes = bincode1::serialize(value).unwrap(); | |
| let (bincode2_result, bytes_read) = | |
| bincode::serde::decode_from_slice( | |
| &bincode1_bytes, | |
| bincode::config::legacy(), | |
| ) | |
| .unwrap(); | |
| assert_eq!( | |
| bytes_read, | |
| bincode1_bytes.len(), | |
| "{} decode did not consume full buffer", | |
| name | |
| ); | |
| assert_eq!(&bincode2_result, value, "{} round-trip mismatch", name); | |
| } |
🤖 Prompt for AI Agents
In `@magicblock-magic-program-api/src/instruction.rs` around lines 205 - 218, The
compatibility helper assert_bincode_deserialize_compatible currently discards
the bytes-read from bincode::serde::decode_from_slice which can hide
trailing-byte mismatches; update the call to capture the returned tuple
(bincode2_result, bytes_read) from decode_from_slice on bincode1_bytes, unwrap
that result, then assert bytes_read == bincode1_bytes.len() (with a clear
message) before asserting bincode2_result equals value so you ensure full buffer
consumption; refer to the function assert_bincode_deserialize_compatible and
variables bincode1_bytes and bincode2_result.
| /// Helper to verify bincode 2 (legacy config) produces identical bytes to bincode 1.3 | ||
| fn assert_bincode_compatible<T: Serialize>(value: &T, name: &str) { | ||
| let bincode2_bytes = | ||
| bincode::serde::encode_to_vec(value, bincode::config::legacy()) | ||
| .unwrap(); | ||
| let bincode1_bytes = bincode1::serialize(value).unwrap(); | ||
| assert_eq!( | ||
| bincode2_bytes, bincode1_bytes, | ||
| "{} serialization mismatch:\nbincode2: {:?}\nbincode1: {:?}", | ||
| name, bincode2_bytes, bincode1_bytes | ||
| ); | ||
| } | ||
|
|
||
| /// Helper to verify bincode 2 can deserialize bincode 1 serialized data | ||
| fn assert_bincode_deserialize_compatible<T>(value: &T, name: &str) | ||
| where | ||
| T: Serialize + for<'de> Deserialize<'de> + PartialEq + std::fmt::Debug, | ||
| { | ||
| let bincode1_bytes = bincode1::serialize(value).unwrap(); | ||
| let bincode2_result: T = bincode::serde::decode_from_slice( | ||
| &bincode1_bytes, | ||
| bincode::config::legacy(), | ||
| ) | ||
| .unwrap() | ||
| .0; | ||
| assert_eq!(&bincode2_result, value, "{} round-trip mismatch", name); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting these helper functions to a shared test utility module.
The assert_bincode_compatible and assert_bincode_deserialize_compatible helper functions are duplicated between this file and magicblock-magic-program-api/src/instruction.rs (as shown in the relevant code snippets). Consider extracting them to a shared test utilities module to reduce duplication and ensure consistent testing patterns across the codebase.
🤖 Prompt for AI Agents
In `@programs/magicblock/src/magic_context.rs` around lines 89 - 115, The two
duplicated helpers assert_bincode_compatible and
assert_bincode_deserialize_compatible should be extracted into a shared test
utility module (e.g., tests/utils or a new crate/test-only module) and imported
from both magic_context.rs and magicblock-magic-program-api/src/instruction.rs;
move the functions (keeping their signatures and trait bounds: T: Serialize and
T: Serialize + Deserialize + PartialEq + Debug respectively), make them pub (or
pub(crate)/pub(super) as needed), update both files to remove the local copies
and call the shared functions, and add any necessary dev-dependencies or module
path imports so both test targets can access the new utility.
| $(COMMITTOR_PROGRAM_SO): $(COMMITTOR_PROGRAM_SRC) | ||
| cargo build-sbf --manifest-path $(COMMITTOR_PROGRAM_DIR)/Cargo.toml \ | ||
| --sbf-out-dir $(ROOT_DEPLOY_DIR)/ | ||
| --sbf-out-dir $(ROOT_DEPLOY_DIR)/ \ | ||
| -- --ignore-rust-version |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Inconsistent indentation in continuation lines.
Lines 207-208 use single-tab indentation while the other build rules (lines 195-196, 199-200, 203-204) use double-tab indentation for continuation. Consider aligning for consistency.
🔧 Suggested fix
$(COMMITTOR_PROGRAM_SO): $(COMMITTOR_PROGRAM_SRC)
cargo build-sbf --manifest-path $(COMMITTOR_PROGRAM_DIR)/Cargo.toml \
- --sbf-out-dir $(ROOT_DEPLOY_DIR)/ \
- -- --ignore-rust-version
+ --sbf-out-dir $(ROOT_DEPLOY_DIR)/ \
+ -- --ignore-rust-version📝 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.
| $(COMMITTOR_PROGRAM_SO): $(COMMITTOR_PROGRAM_SRC) | |
| cargo build-sbf --manifest-path $(COMMITTOR_PROGRAM_DIR)/Cargo.toml \ | |
| --sbf-out-dir $(ROOT_DEPLOY_DIR)/ | |
| --sbf-out-dir $(ROOT_DEPLOY_DIR)/ \ | |
| -- --ignore-rust-version | |
| $(COMMITTOR_PROGRAM_SO): $(COMMITTOR_PROGRAM_SRC) | |
| cargo build-sbf --manifest-path $(COMMITTOR_PROGRAM_DIR)/Cargo.toml \ | |
| --sbf-out-dir $(ROOT_DEPLOY_DIR)/ \ | |
| -- --ignore-rust-version |
🤖 Prompt for AI Agents
In `@test-integration/Makefile` around lines 205 - 208, The Makefile rule for
target $(COMMITTOR_PROGRAM_SO) has inconsistent continuation-line indentation
(the two continuation lines currently use a single tab); update those
continuation lines in that rule to use the same double-tab indentation as the
other build rules so the recipe formatting is consistent (i.e., change the
single-tab before the cargo build-sbf line and the subsequent continuation line
to double tabs in the $(COMMITTOR_PROGRAM_SO) rule).
2 participants
Summary
Switched magicblock-magic-program-api and magicblock-program from bincode 1.3.3 to bincode 2.0.1 to enable no_std support in the
API crate. All other workspace crates remain on bincode 1.x.
Compatibility
Testing
Checklist
Summary by CodeRabbit
Dependencies
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.