Skip to content

fix: Vec<u8> for potentially non-ascii data#323

Draft
joe-p wants to merge 20 commits intomainfrom
fix/block_logs
Draft

fix: Vec<u8> for potentially non-ascii data#323
joe-p wants to merge 20 commits intomainfrom
fix/block_logs

Conversation

@joe-p
Copy link
Collaborator

@joe-p joe-p commented Jan 16, 2026

This fixes errors that were thrown when trying to encode non-ascii data as String

It should be noted that this PR does introduce unsafe code because we need to encode the data as string, but the serde serailizer can only encode a string from a &str, thus we must convert a Vec<u8> to a &str to preserve all the data.

@joe-p joe-p requested a review from lempira as a code owner January 16, 2026 19:59
Copilot AI review requested due to automatic review settings January 16, 2026 19:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes errors that occurred when trying to encode non-ASCII data as String in MessagePack deserialization. The solution changes several field types from String to Vec<u8> and implements custom serde handling for MessagePack strings that may contain non-UTF-8 data.

Changes:

  • Added custom serde module msgpack_string_bytes to handle MessagePack strings as raw bytes
  • Changed BlockStateDelta from a type alias to a struct with custom serialization to support Vec<u8> keys
  • Updated field types in BlockEvalDelta and BlockAppEvalDelta from String/Vec<String> to Vec<u8>/Vec<Vec<u8>>

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
crates/algod_client/src/msgpack_string_bytes.rs New serde module for deserializing msgpack strings as raw bytes
crates/algod_client/src/models/block_state_delta.rs Changed from type alias to struct with custom serialization for byte-keyed HashMap
crates/algod_client/src/models/block_eval_delta.rs Changed bytes field from Option<String> to Option<Vec<u8>>
crates/algod_client/src/models/block_app_eval_delta.rs Changed logs field from Option<Vec<String>> to Option<Vec<Vec<u8>>>
crates/algod_client/src/lib.rs Added msgpack_string_bytes module export
api/oas_generator/rust_oas_generator/templates/models/block/block_state_delta.rs.j2 Template file mirroring changes to block_state_delta.rs
api/oas_generator/rust_oas_generator/templates/models/block/block_eval_delta.rs.j2 Template file mirroring changes to block_eval_delta.rs
api/oas_generator/rust_oas_generator/templates/models/block/block_application_eval_delta.rs.j2 Deleted file (appears to be unused)
api/oas_generator/rust_oas_generator/templates/models/block/block_app_eval_delta.rs.j2 Template file mirroring changes to block_app_eval_delta.rs
api/oas_generator/rust_oas_generator/templates/base/msgpack_string_bytes.rs.j2 Template file for msgpack_string_bytes module
api/oas_generator/rust_oas_generator/templates/base/lib.rs.j2 Template file mirroring changes to lib.rs
api/oas_generator/rust_oas_generator/generator/template_engine.py Added generation logic for msgpack_string_bytes.rs

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@joe-p joe-p marked this pull request as draft January 31, 2026 21:35
@joe-p joe-p marked this pull request as ready for review January 31, 2026 21:58
@joe-p joe-p marked this pull request as draft January 31, 2026 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants