Skip to content

fix: encoder no longer treats all pointer fields as optional<T>#10

Merged
kuny0707 merged 2 commits into
masterfrom
fix/pointer-optional-serialization
Jul 7, 2026
Merged

fix: encoder no longer treats all pointer fields as optional<T>#10
kuny0707 merged 2 commits into
masterfrom
fix/pointer-optional-serialization

Conversation

@ety001

@ety001 ety001 commented Jul 6, 2026

Copy link
Copy Markdown
Member

Problem

The reflection-based encoder in encodeStruct() was adding a 0x01 "pointer present" flag before every pointer field, assuming all Go pointers map to Steem C++ optional<T>. This is incorrect — many operations use plain (non-optional) structs.

For example, witness_update_operation::props is a plain chain_properties in C++, but the Go type uses *ChainProperties. The extra 0x01 byte corrupted the transaction digest, causing steemd to reject the signed transaction with missing required active authority.

Root Cause

In encoder/encoder.go encodeStruct():

// OLD: all pointer fields get a presence byte
if field.Kind() == reflect.Ptr {
    if field.IsNil() { encoder.EncodeNumber(uint8(0)); continue }
    encoder.EncodeNumber(uint8(1))  // ← erroneous flag
    fieldValue = field.Elem().Interface()
}

Fix

  • Add a steem:"optional" struct tag to distinguish truly optional fields
  • Only emit the presence byte for fields tagged steem:"optional"
  • Untagged pointers serialize directly (follow pointer, no flag)
  • Tagged AccountUpdateOperation and AccountUpdate2Operation authority fields as optional (matching their C++ optional<authority> definition)
  • All other pointer fields (witness_update.props, account_create.owner/active/posting, reset_account.new_owner_authority, escrow_transfer.ratification_deadline/escrow_expiration, limit_order_create.expiration, create_proposal.start_date/end_date, pow.*) are left untagged — they are plain structs in C++

Verification

$ go test ./...
ok  github.com/steemit/steemutil	0.011s
ok  github.com/steemit/steemutil/protocol	0.010s
ok  github.com/steemit/steemutil/transaction	0.048s
...

Added TestWitnessUpdateSerialization — verifies the serialized output is 131 bytes (not 132) and that the byte after block_signing_key is NOT 0x01.

ety001 added 2 commits July 6, 2026 22:03
The reflection-based encoder in encodeStruct() was adding a 0x01
"pointer present" flag before every pointer field, assuming all Go
pointers map to Steem C++ optional<T>. This is wrong: many operations
use plain (non-optional) structs like chain_properties and authority.

For example, witness_update_operation::props is a plain chain_properties
in C++, but the Go type uses *ChainProperties. The extra 0x01 byte
corrupted the transaction digest, causing steemd to reject the signed
transaction with "missing required active authority".

Fix:
- Add a steem:"optional" struct tag to distinguish truly optional fields
- In encodeStruct(), only emit the presence byte for tagged fields
- Untagged pointers are serialized directly (follow the pointer, no flag)
- Tagged AccountUpdateOperation and AccountUpdate2Operation authority
  fields as optional (matching their C++ optional<authority> definition)
- Added TestWitnessUpdateSerialization to guard against regression
- TestWitnessUpdateGoldenHex: byte-exact comparison against a hardcoded
  golden hex string, catching any field encoding regression (not just
  the 0x01 pointer bug)
- TestAccountUpdateOptionalFields: verifies optional fields (owner/posting
  nil → 0x00, active set → 0x01) still emit presence flags correctly
@kuny0707
kuny0707 merged commit 8f87bfe into master Jul 7, 2026
3 checks passed
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