fix: use proper serialization for versioned messages in get_fee_for_m… by swarna1101 · Pull Request #7719 · anza-xyz/agave

swarna1101 · 2025-08-26T07:26:33Z

Fix versioned message serialization in `get_fee_for_message`

Problem

get_fee_for_message was failing with "index out of bounds" errors when used with v0 messages containing Address Lookup Tables. The root cause was that bincode::serialize() doesn't include the MESSAGE_VERSION_PREFIX (0x80) required for v0 messages, causing the RPC endpoint to misinterpret the serialized message format.

Solution

Added a serialize_message() method to the SerializableMessage trait that uses the proper native serialization for each message type. For v0 messages, this includes the required version prefix, while legacy messages remain unchanged. Updated get_fee_for_message() to use this new serialization method instead of bincode::serialize().

The fix ensures that both legacy messages and v0 messages with Address Lookup Tables are properly serialized when calling get_fee_for_message(). Added comprehensive tests to verify the serialization behavior and ensure no regressions.

Closes #7563

Can you pls check this @KirillLykov @0xbrw

0xbrw · 2025-08-26T19:16:49Z

@KirillLykov would you know the right person to review this?

KirillLykov · 2025-08-27T19:16:25Z

@steveluscher probably is the best one

KirillLykov · 2025-08-27T19:29:08Z

Also this PR seems to fix the problem, it introduces some additional code. I wonder if it is possible to reuse serialize_and_encode: it calls let serialized = serialize(input) and this input should implement serde::Serialize. It seems that both are implementing serde::Serialize, so it should call correctly method serialize of each. But maybe I don't get something

swarna1101 · 2025-08-28T06:32:16Z

Also this PR seems to fix the problem, it introduces some additional code. I wonder if it is possible to reuse serialize_and_encode: it calls let serialized = serialize(input) and this input should implement serde::Serialize. It seems that both are implementing serde::Serialize, so it should call correctly method serialize of each. But maybe I don't get something

Good point! The issue is that serialize_and_encode uses bincode::serialize(input) which calls the serde serialization trait, but for v0 messages we need the native serialize() method that includes the MESSAGE_VERSION_PREFIX.

The key difference:

bincode::serialize(&v0_message) → No version prefix ❌ (causes "index out of bounds")
v0_message.serialize() → Includes MESSAGE_VERSION_PREFIX ✅ (works correctly)

We could modify serialize_and_encode to detect message types, but this would break its generic nature and risk affecting other use cases. This approach keeps the existing function unchanged while being explicit about using the proper serialization method for each message type.

KirillLykov · 2025-08-28T09:20:36Z

@swarna1101 Hm, I got you.
I wonder if there is a way to reuse here the code from https://github.com/anza-xyz/solana-sdk/blob/master/message/src/versions/mod.rs#L179 instead of introducing the new one?

PS CI fails with extra whitespaces: git diff origin/master --check --oneline

swarna1101 · 2025-08-28T10:32:42Z

Hm, I got you. I wonder if there is a way to reuse here the code from https://github.com/anza-xyz/solana-sdk/blob/master/message/src/versions/mod.rs#L179 instead of introducing the new one?

PS CI fails with extra whitespaces: git diff origin/master --check --oneline

I've refactored to use the existing VersionedMessage from solana-sdk/message/src/versions/mod.rs#L179 instead of the custom trait. It uses proven serialization logic that handles MESSAGE_VERSION_PREFIX. Removed the unnecessary SerializableMessage trait entirely and updated get_fee_for_message() to work directly with VersionedMessage. Tests pass and whitespace issues fixed. 👍

KirillLykov · 2025-08-28T13:24:10Z

    Ok(encoded)
 }

+fn serialize_and_encode_versioned_message(


Are you sure that we need a new serialize_and_encode_versioned_message function? It looks to me serialize_and_encode will work, won't it?

KirillLykov · 2025-08-28T13:25:33Z

+
+    #[test]
+    fn test_versioned_message_serialization_includes_prefix() {
+        use solana_message::{v0, Message as LegacyMessage, compiled_instruction::CompiledInstruction, v0::MessageAddressTableLookup, MessageHeader};


I think we typically avoid using function-level imports.

Removed all function-level use statements from the test functions since the required types are already imported in the test module's use block

KirillLykov · 2025-08-28T13:26:09Z

+    fn test_versioned_message_serialization_includes_prefix() {
+        use solana_message::{v0, Message as LegacyMessage, compiled_instruction::CompiledInstruction, v0::MessageAddressTableLookup, MessageHeader};
+
+        // Test that VersionedMessage serialization includes MESSAGE_VERSION_PREFIX


I would move this comment to be for the whole test, so above #[test]

KirillLykov · 2025-08-28T13:32:32Z

+        message: &VersionedMessage,
    ) -> ClientResult<u64> {
-        let serialized_encoded = serialize_and_encode(message, UiTransactionEncoding::Base64)?;
+        let serialized_encoded = serialize_and_encode_versioned_message(message, UiTransactionEncoding::Base64)?;


Do you know if there are tests for this get_fee_for_message function? If they exist and easy to add a new one, I think it might be valuable to have tests for this function instead of serialization of VersionedMessage because the later lives in sdk.

You're right that we should test the actual get_fee_for_message function rather than just serialization since VersionedMessage is already well-tested in the SDK.

Currently there aren't specific tests for get_fee_for_message - adding proper ones would need either integration tests with a real RPC server or mock-based tests.

Since this PR focuses on the serialization bug fix and our tests do validate the fix works (v0 messages get proper version prefix), I think they serve their purpose here. But I agree comprehensive function tests would be valuable as a follow-up.

Would you prefer adding mock tests in this PR or keeping this focused and doing function tests separately?

I would add a unit test with mock rpc as part of the bug fix (this one) but for backporting it to 2.3.x, it is might be better to make a follow up so this PR is without tests.

got it, will be great if we can create an issue to track it, happy to do that

mergify · 2025-08-29T16:00:01Z

Backports to the stable branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule.

steveluscher · 2025-08-29T20:36:25Z

@jstarry, do you still have the versioned message stuff loaded into your head enough that this is a quick review for you?

mergify · 2025-09-03T12:58:39Z

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

steveluscher

Your original approach of trying to teach SerializableMessage to do the right thing was less intrusive, and required fewer changes. I think the current version of this PR where you force folks to use get_fee_for_message_with_legacy sometimes and get_fee_for_message other times is worse.

With a few tweaks, wouldn't your original change work?

diff --git a/rpc-client/src/nonblocking/rpc_client.rs b/rpc-client/src/nonblocking/rpc_client.rs
index e4e1396abc..293228b07b 100644
--- a/rpc-client/src/nonblocking/rpc_client.rs
+++ b/rpc-client/src/nonblocking/rpc_client.rs
@@ -4671,7 +4671,8 @@ impl RpcClient {
         &self,
         message: &impl SerializableMessage,
     ) -> ClientResult<u64> {
-        let serialized_encoded = serialize_and_encode(message, UiTransactionEncoding::Base64)?;
+        let serialized = message.serialize();
+        let serialized_encoded = BASE64_STANDARD.encode(serialized);
         let result = self
             .send::<Response<Option<u64>>>(
                 RpcRequest::GetFeeForMessage,
diff --git a/rpc-client/src/rpc_client.rs b/rpc-client/src/rpc_client.rs
index bf42e2fb06..567fc44ba0 100644
--- a/rpc-client/src/rpc_client.rs
+++ b/rpc-client/src/rpc_client.rs
@@ -62,9 +62,19 @@ impl RpcClientConfig {
 
 /// Trait used to add support for versioned messages to RPC APIs while
 /// retaining backwards compatibility
-pub trait SerializableMessage: Serialize {}
-impl SerializableMessage for LegacyMessage {}
-impl SerializableMessage for v0::Message {}
+pub trait SerializableMessage {
+    fn serialize(&self) -> Vec<u8>;
+}
+impl SerializableMessage for LegacyMessage {
+    fn serialize(&self) -> Vec<u8> {
+        self.serialize()
+    }
+}
+impl SerializableMessage for v0::Message {
+    fn serialize(&self) -> Vec<u8> {
+        self.serialize()
+    }
+}
 
 /// Trait used to add support for versioned transactions to RPC APIs while
 /// retaining backwards compatibility
@@ -3803,6 +3813,7 @@ mod tests {
         serde_json::{json, Number},
         solana_account_decoder::encode_ui_account,
         solana_account_decoder_client_types::UiAccountEncoding,
+        solana_hash::Hash,
         solana_instruction::error::InstructionError,
         solana_keypair::Keypair,
         solana_rpc_client_api::client_error::ErrorKind,

jstarry · 2025-09-04T03:53:19Z

Ugh, looks like I introduced this bug in solana-labs#28217 because I didn't add proper tests, apologies!

I think that we should do a few things:

impl SerializableMessage for VersionedMessage
Add fn version_prefix() -> Option<u8> to SerializableMessage and impl it for v0::Message
Update fn get_fee_for_message to first serialize the version prefix if is_some

jstarry · 2025-09-04T03:54:18Z

@steveluscher I don't really understand how your suggestion would fix the issue. Where is the version prefix handled?

steveluscher · 2025-09-04T15:37:52Z

@steveluscher I don't really understand how your suggestion would fix the issue. Where is the version prefix handled?

Here's a one-liner that applies the patch, after which you can hyperclick around the calls to serialize() in the SerializableMessage part. That'll lead you right to it.

cat << EOF | git apply
diff --git a/rpc-client/src/nonblocking/rpc_client.rs b/rpc-client/src/nonblocking/rpc_client.rs
index e4e1396abc..293228b07b 100644
--- a/rpc-client/src/nonblocking/rpc_client.rs
+++ b/rpc-client/src/nonblocking/rpc_client.rs
@@ -4671,7 +4671,8 @@ impl RpcClient {
         &self,
         message: &impl SerializableMessage,
     ) -> ClientResult<u64> {
-        let serialized_encoded = serialize_and_encode(message, UiTransactionEncoding::Base64)?;
+        let serialized = message.serialize();
+        let serialized_encoded = BASE64_STANDARD.encode(serialized);
         let result = self
             .send::<Response<Option<u64>>>(
                 RpcRequest::GetFeeForMessage,
diff --git a/rpc-client/src/rpc_client.rs b/rpc-client/src/rpc_client.rs
index bf42e2fb06..567fc44ba0 100644
--- a/rpc-client/src/rpc_client.rs
+++ b/rpc-client/src/rpc_client.rs
@@ -62,9 +62,19 @@ impl RpcClientConfig {
 
 /// Trait used to add support for versioned messages to RPC APIs while
 /// retaining backwards compatibility
-pub trait SerializableMessage: Serialize {}
-impl SerializableMessage for LegacyMessage {}
-impl SerializableMessage for v0::Message {}
+pub trait SerializableMessage {
+    fn serialize(&self) -> Vec<u8>;
+}
+impl SerializableMessage for LegacyMessage {
+    fn serialize(&self) -> Vec<u8> {
+        self.serialize()
+    }
+}
+impl SerializableMessage for v0::Message {
+    fn serialize(&self) -> Vec<u8> {
+        self.serialize()
+    }
+}
 
 /// Trait used to add support for versioned transactions to RPC APIs while
 /// retaining backwards compatibility
@@ -3803,6 +3813,7 @@ mod tests {
         serde_json::{json, Number},
         solana_account_decoder::encode_ui_account,
         solana_account_decoder_client_types::UiAccountEncoding,
+        solana_hash::Hash,
         solana_instruction::error::InstructionError,
         solana_keypair::Keypair,
         solana_rpc_client_api::client_error::ErrorKind,
EOF

swarna1101 · 2025-09-04T15:57:25Z

Thanks a lot for all this discussions, I am checking them, let me get back

steveluscher · 2025-09-17T22:49:09Z

This PR has been rewritten, and a tight test case added to assert that what we're sending to getFeeForMessage is the correct bytes. Ready for review.

…essage

KirillLykov · 2025-09-18T06:20:15Z

But it fails CI

Co-authored-by: kirill lykov <lykov.kirill@gmail.com>

codecov-commenter · 2025-09-18T23:45:18Z

Codecov Report

❌ Patch coverage is 92.85714% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.9%. Comparing base (b984691) to head (ae09fcc).
⚠️ Report is 2170 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #7719   +/-   ##
=======================================
  Coverage    82.9%    82.9%           
=======================================
  Files         823      823           
  Lines      360694   360763   +69     
=======================================
+ Hits       299159   299237   +78     
+ Misses      61535    61526    -9

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

KirillLykov · 2025-09-21T08:09:12Z

        &self,
        message: &impl SerializableMessage,
    ) -> ClientResult<u64> {
-        let serialized_encoded = serialize_and_encode(message, UiTransactionEncoding::Base64)?;


Isn't new code does the same as the function serialize_and_encode but just in place? I wonder if we even need this function if it is a 2 lines of code.

Not exactly. serialize_and_encode does bincode::serialize(thing) whereas the change does thing.serialize(). I'm always happy inlining code, but that's a matter for a different PR.

KirillLykov

the change itself looks good to me, left comment about the function serialize_and_encode.

I am steveluscher.

#7719) * fix: use proper serialization for versioned messages in get_fee_for_message * Clippy satiated Co-authored-by: kirill lykov <lykov.kirill@gmail.com> * Try harder to satiate clippy --------- Co-authored-by: Steven Luscher <steveluscher@users.noreply.github.com> Co-authored-by: kirill lykov <lykov.kirill@gmail.com> (cherry picked from commit 3f25767)

…_for_m… (backport of #7719) (#8138) fix: use proper serialization for versioned messages in get_fee_for_m… (#7719) * fix: use proper serialization for versioned messages in get_fee_for_message * Clippy satiated * Try harder to satiate clippy --------- (cherry picked from commit 3f25767) Co-authored-by: Swarna <swarnabhasinha@gmail.com> Co-authored-by: Steven Luscher <steveluscher@users.noreply.github.com> Co-authored-by: kirill lykov <lykov.kirill@gmail.com>

…_for_m… (backport of #7719) (#8139) fix: use proper serialization for versioned messages in get_fee_for_m… (#7719) * fix: use proper serialization for versioned messages in get_fee_for_message * Clippy satiated * Try harder to satiate clippy --------- (cherry picked from commit 3f25767) Co-authored-by: Swarna <swarnabhasinha@gmail.com> Co-authored-by: Steven Luscher <steveluscher@users.noreply.github.com> Co-authored-by: kirill lykov <lykov.kirill@gmail.com>

mergify Bot added community need:merge-assist labels Aug 26, 2025

mergify Bot requested a review from a team August 26, 2025 07:27

KirillLykov added the CI Pull Request is ready to enter CI label Aug 27, 2025

anza-team removed the CI Pull Request is ready to enter CI label Aug 27, 2025

KirillLykov reviewed Aug 28, 2025

View reviewed changes

KirillLykov added the v2.3 label Aug 29, 2025

steveluscher requested a review from jstarry August 29, 2025 20:36

KirillLykov added the CI Pull Request is ready to enter CI label Sep 3, 2025

anza-team removed the CI Pull Request is ready to enter CI label Sep 3, 2025

KirillLykov added the CI Pull Request is ready to enter CI label Sep 3, 2025

anza-team removed the CI Pull Request is ready to enter CI label Sep 3, 2025

KirillLykov added the v3.0 label Sep 3, 2025

steveluscher previously requested changes Sep 3, 2025

View reviewed changes

steveluscher self-assigned this Sep 16, 2025

steveluscher force-pushed the fix/versioned-message-serialization-get-fee branch 2 times, most recently from 9904aa8 to 5c486b2 Compare September 17, 2025 22:49

steveluscher requested review from KirillLykov, alannza and joncinque September 17, 2025 22:49

fix: use proper serialization for versioned messages in get_fee_for_m…

cb41b5c

…essage

steveluscher force-pushed the fix/versioned-message-serialization-get-fee branch from 5c486b2 to cb41b5c Compare September 17, 2025 22:50

KirillLykov previously approved these changes Sep 18, 2025

View reviewed changes

KirillLykov reviewed Sep 18, 2025

View reviewed changes

Comment thread rpc-client/src/rpc_client.rs Outdated

Clippy satiated

bdb61f0

Co-authored-by: kirill lykov <lykov.kirill@gmail.com>

steveluscher dismissed KirillLykov’s stale review via bdb61f0 September 18, 2025 22:54

steveluscher reviewed Sep 18, 2025

View reviewed changes

Comment thread rpc-client/src/rpc_client.rs Outdated

Try harder to satiate clippy

ae09fcc

steveluscher requested a review from KirillLykov September 20, 2025 06:20

KirillLykov reviewed Sep 21, 2025

View reviewed changes

KirillLykov approved these changes Sep 21, 2025

View reviewed changes

steveluscher self-requested a review September 22, 2025 18:34

steveluscher merged commit 3f25767 into anza-xyz:master Sep 22, 2025
54 checks passed

This was referenced Sep 22, 2025

v2.3: fix: use proper serialization for versioned messages in get_fee_for_m… (backport of #7719) #8138

Merged

v3.0: fix: use proper serialization for versioned messages in get_fee_for_m… (backport of #7719) #8139

Merged

greptile-apps Bot mentioned this pull request Jan 12, 2026

release: 2.0.2 solana-foundation/kora#299

Merged

Conversation

swarna1101 commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix versioned message serialization in get_fee_for_message

Problem

Solution

Uh oh!

0xbrw commented Aug 26, 2025

Uh oh!

KirillLykov commented Aug 27, 2025

Uh oh!

KirillLykov commented Aug 27, 2025

Uh oh!

swarna1101 commented Aug 28, 2025

Uh oh!

KirillLykov commented Aug 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

swarna1101 commented Aug 28, 2025

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!

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!

mergify Bot commented Aug 29, 2025

Uh oh!

steveluscher commented Aug 29, 2025

Uh oh!

mergify Bot commented Sep 3, 2025

Uh oh!

steveluscher left a comment

Choose a reason for hiding this comment

Uh oh!

jstarry commented Sep 4, 2025

Uh oh!

jstarry commented Sep 4, 2025

Uh oh!

steveluscher commented Sep 4, 2025

Uh oh!

swarna1101 commented Sep 4, 2025

Uh oh!

steveluscher commented Sep 17, 2025

Uh oh!

KirillLykov commented Sep 18, 2025

Uh oh!

Uh oh!

Uh oh!

codecov-commenter commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KirillLykov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

swarna1101 commented Aug 26, 2025 •

edited

Loading

Fix versioned message serialization in `get_fee_for_message`

KirillLykov commented Aug 28, 2025 •

edited

Loading

codecov-commenter commented Sep 18, 2025 •

edited

Loading