Add Test with TS SDK (same input, same output) #185
+371
−27
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… compatibility modes Added comprehensive tests for Option types including u8, u16, u32, u64, u128, u256, bool, string, and address. Each type includes cases for both standard and compatibility modes, ensuring robust validation of serialization behavior.
Refactored tests for Option types to use direct values instead of hex strings, enhancing clarity and consistency. Adjusted expected outputs for various data types in compatibility mode, ensuring accurate validation of serialization behavior.
- Added support for direct string inputs in ConvertToOption function, enhancing usability for Option types. - Removed the unused convertCompatibilitySerializedType function to streamline the codebase.
Expanded tests for Option types to include string inputs for u8, u16, u32, u64, u128, u256, bool, and address. Added cases for compatibility mode, ensuring accurate serialization behavior for both direct values and string representations. Improved organization of test cases for better clarity.
There was a problem hiding this comment.
Pull Request Overview
Implements Option type compatibility between Go and TypeScript SDKs to ensure consistent serialization behavior. The change aligns Go SDK Option handling with the TS SDK implementation where nil represents None and non-nil values represent Some.
- Removes complex compatibility mode handling for Option types
- Simplifies Option serialization to match TS SDK behavior
- Updates test cases to cover comprehensive Option type scenarios
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| typeConversion.go | Simplifies ConvertToOption function by removing compatibility mode handling and unused helper function |
| typeConversion_test.go | Adds comprehensive test cases for Option types with various inner types (u8, u16, u32, u64, u128, u256, bool, address, string, object, vectors) |
| CHANGELOG.md | Documents the feature addition and refactoring changes |
Comments suppressed due to low confidence (1)
typeConversion.go:537
- The function lacks documentation explaining its purpose, parameters, and behavior. Add a comment describing that it converts Go values to Option type serialization compatible with the TS SDK, where nil represents None and non-nil values represent Some.
func ConvertToOption(typeParam TypeTag, arg any, generics []TypeTag, options ...any) ([]byte, error) {
if arg == nil {
return bcs.SerializeU8(0)
}
b := []byte{1}
buffer, err := ConvertArg(typeParam, arg, generics, options...)
if err != nil {
return nil, err
}
return append(b, buffer...), nil
}
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
0x-j
approved these changes
Sep 11, 2025
gregnazario
reviewed
Sep 11, 2025
…Option Added support for compatibility mode in the ConvertToOption function, allowing string representations of various types (u8, u16, u32, u64, u128, u256, bool, address) to be converted into their respective byte formats. Introduced a new helper function, convertCompatibilitySerializedType, to handle serialization for different type tags, enhancing the overall type conversion process.
WGB5445
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
In the current Go SDK, Option uses the BCS-serialized result for deserialization. The benefit of this approach is that 0x00 can represent none, and 0x01 + the serialized bytes of another type can represent some.
However, this behavior is inconsistent with the TS SDK implementation. As a result, parameters that can be serialized correctly in the TS SDK cannot be deserialized properly in Go.
For example:
• Go: supports 0x00 serialization to represent none
• TS: uses null or undefined to represent none
• Go: uses 0x0101 serialization to represent some(1)
• TS: uses "1" to represent some(1)
A partner recently discovered this inconsistency and requested that we align with the TS SDK behavior.
This PR implements that alignment. One important note: when using the TS SDK, passing "" (empty string) as an argument cannot represent none. In Go, only nil can be used to indicate that the variable is none.
Below are the corresponding TS tests.