fix: make serializeForEntryFunction backward-compatible with older Serializers (DVR-143)#861
fix: make serializeForEntryFunction backward-compatible with older Serializers (DVR-143)#861gregnazario wants to merge 3 commits intomainfrom
Conversation
…rializers When a wallet extension (e.g. Petra) bundles an older SDK version and receives a transaction object built with SDK v6, the v6 objects' serializeForEntryFunction methods call serializer.serializeAsBytes(this) which doesn't exist on older Serializers, causing 'e.serializeAsBytes is not a function' errors. This commit: - Adds serializeEntryFunctionBytesCompat() helper that falls back to the pre-v6 bcsToBytes()+serializeBytes() pattern when serializeAsBytes is not available on the provided serializer - Updates all serializeForEntryFunction implementations (Bool, U8-U256, I8-I256, AccountAddress, MoveVector, MoveString, MoveOption) to use the compat helper - Adds 25 unit tests verifying cross-version serializer compatibility, including full transaction and fee payer transaction serialization Patch from: aptos-labs/aptos-core#19148 Fixes: DVR-143 Co-authored-by: Greg Nazario <greg@gnazar.io>
There was a problem hiding this comment.
Pull request overview
This PR restores cross-version wallet compatibility by making serializeForEntryFunction work with older Serializer implementations that don’t provide serializeAsBytes, preventing failures in fee payer flows after the TS SDK v6.2.0 upgrade.
Changes:
- Added
serializeEntryFunctionBytesCompat()helper that usesserializeAsByteswhen available, otherwise falls back tobcsToBytes() + serializeBytes(). - Updated
serializeForEntryFunctionimplementations (Move primitives/structs andAccountAddress) to use the compat helper. - Added unit tests covering legacy-serializer scenarios and updated the changelog entry.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/walletSerializerCompat.test.ts | Adds unit tests simulating a legacy serializer to ensure identical serialization output across versions. |
| src/core/accountAddress.ts | Routes entry-function serialization through the new compatibility helper. |
| src/bcs/serializer.ts | Introduces serializeEntryFunctionBytesCompat() runtime fallback logic. |
| src/bcs/serializable/moveStructs.ts | Updates complex Move types’ entry-function serialization to use the compat helper. |
| src/bcs/serializable/movePrimitives.ts | Updates Move primitive entry-function serialization to use the compat helper. |
| CHANGELOG.md | Documents the DVR-143 fix and the compatibility fallback behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| */ | ||
| serializeForEntryFunction(serializer: Serializer): void { | ||
| serializer.serializeAsBytes(this); | ||
| serializeEntryFunctionBytesCompat(serializer, this); |
There was a problem hiding this comment.
The JSDoc for serializeForEntryFunction still says it “Uses the optimized serializeAsBytes method”, but the implementation now goes through serializeEntryFunctionBytesCompat() which may fall back to bcsToBytes() + serializeBytes() when a legacy Serializer is provided. Please update the doc comment to reflect the new behavior (prefer serializeAsBytes when available, otherwise fallback for compatibility).
| * @category BCS | ||
| */ | ||
| serializeForEntryFunction(serializer: Serializer): void { | ||
| serializer.serializeAsBytes(this); | ||
| serializeEntryFunctionBytesCompat(serializer, this); | ||
| } |
There was a problem hiding this comment.
The JSDoc here states this path “Uses the optimized serializeAsBytes method”, but the implementation now calls serializeEntryFunctionBytesCompat() which may use serializeAsBytes or a compatibility fallback depending on the Serializer instance. Please adjust the comment to describe the compatibility behavior so it stays accurate.
| * @group Implementation | ||
| * @category BCS | ||
| */ | ||
| serializeForEntryFunction(serializer: Serializer): void { | ||
| serializer.serializeAsBytes(this); | ||
| serializeEntryFunctionBytesCompat(serializer, this); | ||
| } |
There was a problem hiding this comment.
The JSDoc above this method still claims it “Uses the optimized serializeAsBytes method”, but the implementation now uses serializeEntryFunctionBytesCompat() which may fall back to bcsToBytes() + serializeBytes() for older Serializer implementations. Please update the comment to match the new behavior (prefer optimized path when available, otherwise fallback for compatibility).
Update serializeForEntryFunction JSDoc comments in movePrimitives.ts, moveStructs.ts, and accountAddress.ts to accurately describe the new behavior: uses serializeAsBytes when available, with a fallback for older Serializer versions. Co-authored-by: Greg Nazario <greg@gnazar.io>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
Applies the patch from aptos-labs/aptos-core#19148 to fix DVR-143.
After upgrading to TS SDK v6.2.0, wallet extensions like Petra that bundle an older SDK version fail with
Error: e.serializeAsBytes is not a functionwhen using the fee payer (sign + submit) flow. This happens because the v6 performance pass changed allserializeForEntryFunctionmethods to callserializer.serializeAsBytes(this), which doesn't exist on older Serializer implementations.Changes:
serializeEntryFunctionBytesCompat()helper insrc/bcs/serializer.tsthat performs a runtime check: if the serializer hasserializeAsBytes, use it (optimal path); otherwise fall back to the pre-v6bcsToBytes()+serializeBytes()pattern.serializeForEntryFunctionimplementations acrossmovePrimitives.ts,moveStructs.ts, andaccountAddress.tsto use the compat helper.serializeForEntryFunctionmethods to accurately reflect the compatibility fallback behavior.tests/unit/walletSerializerCompat.test.tsverifying cross-version serializer compatibility.Test Plan
serializeAsBytesremoved) and verifying identical output for all Move primitive types, complex types, and full transaction serialization.pnpm check(Biome lint + format) passes cleanly.Related Links
Checklist
pnpm fmt?CHANGELOG.md?