Add protobuf action serialization support#239
Conversation
Enable contracts to use Protocol Buffers as an alternative serialization format for action data. The node-side abi_serializer uses Google's libprotobuf DynamicMessageFactory to serialize/deserialize protobuf action data at runtime, matching CDT's zpp_bits size_varint wire format. Changes: - abi_def: add protobuf_types field (may_not_exist<string>) for ABI v1.3 - abi_serializer: protobuf binary<->variant conversion with double-varint length prefix format compatible with CDT's zpp_bits size_varint - abi_serializer: custom to_variant/from_variant for abi_def to handle protobuf_types as JSON object in ABI files, string internally - Forward-declare protobuf types in header, includes in .cpp only - vcpkg.json + chain CMakeLists: add protobuf dependency - 12 unit tests: ABI loading, round-trip, flattened actions, nested messages, repeated fields, mixed protobuf+regular fields, known-bytes format verification - Integration test (proto_abi_test.py): deploy and push protobuf actions on a live test chain - CLI test: clio pack/unpack round-trip with --abi-file - Documentation (contracts/protocol-buffers.md) Also fixes: move using-declarations above BOOST_AUTO_TEST_SUITE in native_overlay_tests.cpp to prevent std namespace shadowing.
There was a problem hiding this comment.
Pull request overview
Adds node-side Protocol Buffer (protobuf) support for ABI-based action data serialization/deserialization, aligned with CDT’s protobuf generation and wire format.
Changes:
- Adds protobuf dependency + links
libprotobufintosysio_chain. - Extends ABI v1.3 with
protobuf_typesand implements runtime protobuf ↔ JSON(variant) conversion inabi_serializer. - Adds unit/integration tests and documentation for protobuf contract workflows and clio usage.
Reviewed changes
Copilot reviewed 16 out of 22 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| vcpkg.json | Adds protobuf as a dependency for the build. |
| libraries/chain/CMakeLists.txt | Finds/links protobuf::libprotobuf for node-side runtime support. |
| libraries/chain/include/sysio/chain/abi_serializer.hpp | Adds protobuf-related members and helpers to abi_serializer. |
| libraries/chain/include/sysio/chain/abi_def.hpp | Adds protobuf_types to ABI definition + declares custom JSON conversion. |
| libraries/chain/abi_serializer.cpp | Implements protobuf parsing, message conversion, and ABI JSON handling. |
| unittests/abi_tests.cpp | Adds protobuf ABI serialization/deserialization and round-trip tests. |
| unittests/native_overlay_tests.cpp | Refactors conditional compilation to avoid empty-suite failures. |
| unittests/test-contracts/CMakeLists.txt | Registers new protobuf ABI test contract. |
| unittests/test-contracts/proto_abi_test/CMakeLists.txt | Builds protobuf-generated headers for the test contract. |
| unittests/test-contracts/proto_abi_test/test.proto | Defines protobuf messages used for action input/output tests. |
| unittests/test-contracts/proto_abi_test/proto_abi_test.cpp | Adds a test contract using sysio::pb<T> actions. |
| unittests/test-contracts/proto_abi_test/proto_abi_test.abi | Adds ABI v1.3 example embedding protobuf_types. |
| tests/CMakeLists.txt | Installs and registers the new python integration test. |
| tests/proto_abi_test.py | End-to-end node test deploying and invoking protobuf actions. |
| tests/cli_test.py | Adds clio --abi-file pack/unpack tests for protobuf actions. |
| contracts/protocol-buffers.md | Documents protobuf ABI format, wire format, and tooling usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
libraries/chain/abi_serializer.cpp
Outdated
| gpb::util::JsonParseOptions opts; | ||
| opts.ignore_unknown_fields = true; | ||
| auto status = gpb::util::JsonStringToMessage(abi.protobuf_types.value, &fds); | ||
| if( status.ok() ) { | ||
| pb_pool = std::make_shared<gpb::DescriptorPool>(); | ||
| for( int i = 0; i < fds.file_size(); ++i ) { | ||
| pb_pool->BuildFile(fds.file(i)); | ||
| } |
There was a problem hiding this comment.
opts.ignore_unknown_fields = true; is currently not applied because JsonStringToMessage is called without passing opts. This makes parsing stricter than intended and also leaves opts unused. Pass opts into JsonStringToMessage(..., opts) (using the overload that accepts JsonParseOptions).
libraries/chain/abi_serializer.cpp
Outdated
| auto json = fc::json::to_string(var, fc::time_point::maximum()); | ||
| gpb::util::JsonParseOptions opts; | ||
| opts.ignore_unknown_fields = true; | ||
| auto status = gpb::util::JsonStringToMessage(json, &msg); |
There was a problem hiding this comment.
Same issue as above: opts.ignore_unknown_fields is set but not actually used in the JsonStringToMessage call. If unknown-field tolerance is required, pass opts via the appropriate overload.
| auto status = gpb::util::JsonStringToMessage(json, &msg); | |
| auto status = gpb::util::JsonStringToMessage(json, &msg, opts); |
libraries/chain/abi_serializer.cpp
Outdated
| if( status.ok() ) { | ||
| pb_pool = std::make_shared<gpb::DescriptorPool>(); | ||
| for( int i = 0; i < fds.file_size(); ++i ) { | ||
| pb_pool->BuildFile(fds.file(i)); | ||
| } | ||
| pb_factory = std::make_shared<gpb::DynamicMessageFactory>(pb_pool.get()); | ||
| } |
There was a problem hiding this comment.
When protobuf_types is present but JSON parsing fails (or when BuildFile(...) fails), protobuf support silently disables (no pb_pool/pb_factory) and errors will surface later as generic 'unknown type' failures. Consider asserting immediately when protobuf_types is non-empty but cannot be parsed/built, and include the protobuf status.message() / file name in the error to make ABI issues actionable.
| if( status.ok() ) { | |
| pb_pool = std::make_shared<gpb::DescriptorPool>(); | |
| for( int i = 0; i < fds.file_size(); ++i ) { | |
| pb_pool->BuildFile(fds.file(i)); | |
| } | |
| pb_factory = std::make_shared<gpb::DynamicMessageFactory>(pb_pool.get()); | |
| } | |
| SYS_ASSERT( status.ok(), abi_exception, | |
| "Failed to parse protobuf_types JSON: {}", status.message() ); | |
| pb_pool = std::make_shared<gpb::DescriptorPool>(); | |
| for( int i = 0; i < fds.file_size(); ++i ) { | |
| const gpb::FileDescriptor* fd = pb_pool->BuildFile(fds.file(i)); | |
| SYS_ASSERT( fd != nullptr, abi_exception, | |
| "Failed to build protobuf file descriptor for '{}'", fds.file(i).name() ); | |
| } | |
| pb_factory = std::make_shared<gpb::DynamicMessageFactory>(pb_pool.get()); |
| fc::datastream<const char*> inner_stream(stream.pos(), outer_len.value); | ||
| fc::unsigned_int pb_len; | ||
| fc::raw::unpack(inner_stream, pb_len); | ||
|
|
||
| auto prototype = pb_factory->GetPrototype(desc); | ||
| std::unique_ptr<google::protobuf::Message> msg(prototype->New()); | ||
| SYS_ASSERT( msg->ParseFromArray(inner_stream.pos(), pb_len.value), unpack_exception, | ||
| "Failed to parse protobuf message '{}'", impl::limit_size(type) ); | ||
| stream.skip(outer_len.value); |
There was a problem hiding this comment.
pb_len is not validated against the remaining bytes inside the outer_len window before calling ParseFromArray(...). A malformed input could set pb_len larger than inner_stream.remaining(), potentially causing an out-of-bounds read inside protobuf parsing. Add a bounds check (and ideally also validate that pb_len + encoded-size-of-pb_len equals outer_len to reject trailing garbage) before parsing.
libraries/chain/abi_serializer.cpp
Outdated
| fc::reflector<sysio::chain::abi_def>::visit( | ||
| fc::to_variant_visitor<sysio::chain::abi_def>(mvo, abi) | ||
| ); | ||
| // Add protobuf_types as JSON object (not string) |
There was a problem hiding this comment.
Because FC_REFLECT(...)(protobuf_types) is included and may_not_exist<T>’s to_variant appears to always serialize .value, the reflector visit likely emits \"protobuf_types\": \"\" even when empty. That means ABIs without protobuf may still output a protobuf_types field (as an empty string), which contradicts the stated may_not_exist semantics and could break consumers expecting ABI 1.2/1.3 schema differences. Consider explicitly removing/omitting protobuf_types from mvo when abi.protobuf_types.value is empty.
| // Add protobuf_types as JSON object (not string) | |
| // Ensure protobuf_types follows may_not_exist semantics: | |
| // remove the field added by the reflector (which may be an empty string), | |
| // then, if non-empty, re-add it as a JSON object instead of a string. | |
| mvo.erase("protobuf_types"); | |
| // Add protobuf_types as JSON object (not string) when non-empty |
contracts/protocol-buffers.md
Outdated
|
|
||
| ```cmake | ||
| find_package(protobuf CONFIG REQUIRED) | ||
| target_link_libraries(chain PUBLIC protobuf::libprotobuf) |
There was a problem hiding this comment.
The CMake snippet uses target_link_libraries(chain ...), but the actual target in this repo change is sysio_chain. Update the docs to match the real target name to avoid copy/paste build failures.
| target_link_libraries(chain PUBLIC protobuf::libprotobuf) | |
| target_link_libraries(sysio_chain PUBLIC protobuf::libprotobuf) |
| configure_file( ${CMAKE_CURRENT_SOURCE_DIR}/proto_abi_test.wasm ${CMAKE_CURRENT_BINARY_DIR}/proto_abi_test.wasm COPYONLY ) | ||
| configure_file( ${CMAKE_CURRENT_SOURCE_DIR}/proto_abi_test.abi ${CMAKE_CURRENT_BINARY_DIR}/proto_abi_test.abi COPYONLY ) |
There was a problem hiding this comment.
The else() branch unconditionally copies proto_abi_test.wasm from the source directory. In this PR diff, the .abi is added but a corresponding prebuilt .wasm is not shown; if it is missing, CMake configuration will fail when BUILD_TEST_CONTRACTS is OFF. Add/commit the prebuilt WASM artifact (or adjust the fallback behavior) so non-contract builds remain functional.
| configure_file( ${CMAKE_CURRENT_SOURCE_DIR}/proto_abi_test.wasm ${CMAKE_CURRENT_BINARY_DIR}/proto_abi_test.wasm COPYONLY ) | |
| configure_file( ${CMAKE_CURRENT_SOURCE_DIR}/proto_abi_test.abi ${CMAKE_CURRENT_BINARY_DIR}/proto_abi_test.abi COPYONLY ) | |
| if( EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/proto_abi_test.wasm" ) | |
| configure_file( ${CMAKE_CURRENT_SOURCE_DIR}/proto_abi_test.wasm ${CMAKE_CURRENT_BINARY_DIR}/proto_abi_test.wasm COPYONLY ) | |
| else() | |
| message(WARNING "proto_abi_test.wasm not found in ${CMAKE_CURRENT_SOURCE_DIR}; skipping copy of prebuilt WASM artifact.") | |
| endif() | |
| if( EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/proto_abi_test.abi" ) | |
| configure_file( ${CMAKE_CURRENT_SOURCE_DIR}/proto_abi_test.abi ${CMAKE_CURRENT_BINARY_DIR}/proto_abi_test.abi COPYONLY ) | |
| else() | |
| message(WARNING "proto_abi_test.abi not found in ${CMAKE_CURRENT_SOURCE_DIR}; skipping copy of ABI artifact.") | |
| endif() |
There was a problem hiding this comment.
It was added. This is not valid.
| int32 value = 1; | ||
| string str_value = 2; |
There was a problem hiding this comment.
Indentation is inconsistent within ActResult (fields use 4 spaces while the rest of the file uses 2). Align formatting for readability and consistency.
| int32 value = 1; | |
| string str_value = 2; | |
| int32 value = 1; | |
| string str_value = 2; |
…bounds - Pass JsonParseOptions (ignore_unknown_fields) to JsonStringToMessage at both call sites instead of leaving opts unused - Assert immediately when protobuf_types JSON parsing or BuildFile fails instead of silently disabling protobuf support - Validate pb_len against remaining inner stream bytes before ParseFromArray to prevent out-of-bounds reads on malformed input - Omit protobuf_types from ABI variant output when empty to match may_not_exist semantics
Summary
Adds node-side Protocol Buffer support for smart contract action data serialization, complementing the CDT-side support in Wire-Network/wire-cdt#32.
Contracts can now use protobuf as an alternative to CDT's default datastream packing, providing ID-based field encoding, backwards-compatible schema evolution, and language-neutral message definitions.
What changed
Core (
libraries/chain)abi_def: newprotobuf_typesfield (may_not_exist<string>) for ABI version 1.3abi_serializer: runtime protobuf binary↔variant conversion using Google'sDynamicMessageFactory, matching CDT'szpp_bitssize_varintwire format (double-varint length prefix)to_variant/from_variantforabi_defto handleprotobuf_typesas a JSON object in ABI files but stored as a string internally.hpp, includes only in.cppprotobufadded to vcpkg dependencies and linked in chain CMakeListsDocumentation
contracts/protocol-buffers.md: architecture overview, ABI v1.3 format, wire format details, clio usage, and end-to-end walkthrough for creating a protobuf contractABI format
Single
pb<T>parameter actions use a flattened format — the action type points directly at the protobuf type with no wrapper struct:{ "version": "sysio::abi/1.3", "structs": [], "actions": [ { "name": "greet", "type": "protobuf::pbexample.Greeting", "ricardian_contract": "" } ], "protobuf_types": { "file": [ { "name": "pbexample.proto", "package": "pbexample", "messageType": [ { "name": "Greeting", "field": [ { "name": "from", "number": 1, "type": "TYPE_STRING", "label": "LABEL_OPTIONAL" }, { "name": "message", "number": 2, "type": "TYPE_STRING", "label": "LABEL_OPTIONAL" }, { "name": "importance", "number": 3, "type": "TYPE_INT32", "label": "LABEL_OPTIONAL" } ] } ], "syntax": "proto3" } ] } } Action data is flat JSON with no wrapper: clio push action pbexample greet '{"from":"alice","message":"hello","importance":5}' -p pbexample@active Multi-parameter actions generate a wrapper struct with one field per parameter: { "structs": [ { "name": "settle", "fields": [ { "name": "header", "type": "protobuf::mypackage.Header" }, { "name": "body", "type": "protobuf::mypackage.Body" } ] } ], "actions": [ { "name": "settle", "type": "settle" } ] } clio push action mycontract settle '{"header":{"version":1},"body":{"payload":"data"}}' -p myaccount@active