From e0934d6143d7b69d7e2b1615165a09e7475aa372 Mon Sep 17 00:00:00 2001 From: ety001 Date: Mon, 6 Jul 2026 22:03:21 +0800 Subject: [PATCH 1/2] fix: encoder no longer treats all pointer fields as optional 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. 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 definition) - Added TestWitnessUpdateSerialization to guard against regression --- encoder/encoder.go | 27 +++++--- protocol/operations.go | 12 ++-- .../witness_update_serialization_test.go | 65 +++++++++++++++++++ 3 files changed, 89 insertions(+), 15 deletions(-) create mode 100644 transaction/witness_update_serialization_test.go diff --git a/encoder/encoder.go b/encoder/encoder.go index 3edcffe..4619be8 100644 --- a/encoder/encoder.go +++ b/encoder/encoder.go @@ -267,18 +267,27 @@ func (encoder *Encoder) encodeStruct(rv reflect.Value) error { // Handle pointer fields if field.Kind() == reflect.Ptr { + // In Steem's C++ code, some fields are optional (encoded as + // bool + value) while others are plain structs that merely use a + // Go pointer for nil-safety. A struct tag steem:"optional" marks + // the truly optional fields. Untagged pointers are serialized + // directly (following the pointer, no presence byte). + isOptional := fieldType.Tag.Get("steem") == "optional" if field.IsNil() { - // For nil pointers in structs, we need to check if it's optional - // In Steem, optional fields are encoded as: bool (has_value) + value - // If nil, encode false (0) - if err := encoder.EncodeNumber(uint8(0)); err != nil { - return errors.Wrapf(err, "failed to encode nil pointer field %s", fieldType.Name) + if isOptional { + // optional with no value: encode false (0) + if err := encoder.EncodeNumber(uint8(0)); err != nil { + return errors.Wrapf(err, "failed to encode nil optional field %s", fieldType.Name) + } + continue } - continue + return errors.Errorf("non-optional pointer field %s is nil", fieldType.Name) } - // Encode true (1) to indicate value is present, then encode the value - if err := encoder.EncodeNumber(uint8(1)); err != nil { - return errors.Wrapf(err, "failed to encode pointer presence for field %s", fieldType.Name) + if isOptional { + // Encode true (1) to indicate value is present, then encode the value + if err := encoder.EncodeNumber(uint8(1)); err != nil { + return errors.Wrapf(err, "failed to encode optional presence for field %s", fieldType.Name) + } } fieldValue = field.Elem().Interface() } diff --git a/protocol/operations.go b/protocol/operations.go index 9590280..1eb8a41 100644 --- a/protocol/operations.go +++ b/protocol/operations.go @@ -267,9 +267,9 @@ func (op *AccountCreateOperation) Data() any { type AccountUpdateOperation struct { Account string `json:"account"` - Owner *Authority `json:"owner"` - Active *Authority `json:"active"` - Posting *Authority `json:"posting"` + Owner *Authority `json:"owner" steem:"optional"` + Active *Authority `json:"active" steem:"optional"` + Posting *Authority `json:"posting" steem:"optional"` MemoKey string `json:"memo_key"` JsonMetadata string `json:"json_metadata"` } @@ -1220,9 +1220,9 @@ func (op *WitnessSetPropertiesOperation) Data() any { type AccountUpdate2Operation struct { Account string `json:"account"` - Owner *Authority `json:"owner"` - Active *Authority `json:"active"` - Posting *Authority `json:"posting"` + Owner *Authority `json:"owner" steem:"optional"` + Active *Authority `json:"active" steem:"optional"` + Posting *Authority `json:"posting" steem:"optional"` MemoKey string `json:"memo_key"` JsonMetadata string `json:"json_metadata"` PostingJsonMetadata string `json:"posting_json_metadata"` diff --git a/transaction/witness_update_serialization_test.go b/transaction/witness_update_serialization_test.go new file mode 100644 index 0000000..05810d5 --- /dev/null +++ b/transaction/witness_update_serialization_test.go @@ -0,0 +1,65 @@ +package transaction + +import ( + "encoding/hex" + "testing" + "time" + + "github.com/steemit/steemutil/protocol" +) + +// TestWitnessUpdateSerialization verifies that WitnessUpdateOperation serializes +// the props field as a plain struct WITHOUT an erroneous 0x01 "pointer present" +// flag. The bug was that the encoder treated all pointer fields as optional. +func TestWitnessUpdateSerialization(t *testing.T) { + expiration := time.Date(2026, 7, 6, 11, 31, 28, 0, time.UTC) + + op := &protocol.WitnessUpdateOperation{ + Owner: "ety001", + URL: "https://steem.fans", + BlockSigningKey: "STM7cKkyBCxTRkdxoGSP5ypagNeaVYeR7oWcBsNfKurXg5jnAtNqm", + Props: &protocol.ChainProperties{ + AccountCreationFee: "3.000 STEEM", + MaximumBlockSize: 65536, + SBDInterestRate: 0, + }, + Fee: "0.000 STEEM", + } + + stx := NewSignedTransaction(&Transaction{ + RefBlockNum: 18668, + RefBlockPrefix: 2441809904, + Expiration: &protocol.Time{Time: &expiration}, + Extensions: []interface{}{}, + }) + stx.PushOperation(op) + + serialized, err := stx.Serialize() + if err != nil { + t.Fatalf("Serialize failed: %v", err) + } + + // The bug produced 132 bytes (extra 0x01 before props). Correct is 131. + if len(serialized) != 131 { + t.Errorf("expected 131 bytes, got %d bytes (hex: %s)", len(serialized), hex.EncodeToString(serialized)) + } + + // The byte right after block_signing_key should NOT be 0x01 (pointer-present flag). + // It should be the first byte of the ChainProperties.account_creation_fee asset (0xb8). + keyStr := "STM7cKkyBCxTRkdxoGSP5ypagNeaVYeR7oWcBsNfKurXg5jnAtNqm" + keyEnd := -1 + for i := 0; i < len(serialized)-len(keyStr); i++ { + if string(serialized[i:i+len(keyStr)]) == keyStr { + keyEnd = i + len(keyStr) + break + } + } + if keyEnd == -1 { + t.Fatalf("could not find block_signing_key in serialized output") + } + + if serialized[keyEnd] == 0x01 { + t.Errorf("found erroneous 0x01 pointer-present flag at offset %d; "+ + "witness_update props must serialize as plain struct, not optional", keyEnd) + } +} From 19a6d6421cacef3bfeb2ce2b19a668ca661de714 Mon Sep 17 00:00:00 2001 From: ety001 Date: Mon, 6 Jul 2026 22:46:58 +0800 Subject: [PATCH 2/2] test: add golden hex comparison and account_update optional path test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- .../witness_update_serialization_test.go | 104 ++++++++++++++---- 1 file changed, 85 insertions(+), 19 deletions(-) diff --git a/transaction/witness_update_serialization_test.go b/transaction/witness_update_serialization_test.go index 05810d5..1529312 100644 --- a/transaction/witness_update_serialization_test.go +++ b/transaction/witness_update_serialization_test.go @@ -8,10 +8,14 @@ import ( "github.com/steemit/steemutil/protocol" ) -// TestWitnessUpdateSerialization verifies that WitnessUpdateOperation serializes -// the props field as a plain struct WITHOUT an erroneous 0x01 "pointer present" -// flag. The bug was that the encoder treated all pointer fields as optional. -func TestWitnessUpdateSerialization(t *testing.T) { +// TestWitnessUpdateGoldenHex verifies the exact byte-level serialization of a +// witness_update transaction. The golden hex was captured from the fixed encoder +// and validated against steemd's expected C++ FC_REFLECT serialization. +// +// The regression this guards against: the encoder used to insert an erroneous +// 0x01 "pointer-present" byte before ChainProperties (a plain, non-optional +// struct), producing 132 bytes instead of 131 and corrupting the digest. +func TestWitnessUpdateGoldenHex(t *testing.T) { expiration := time.Date(2026, 7, 6, 11, 31, 28, 0, time.UTC) op := &protocol.WitnessUpdateOperation{ @@ -39,27 +43,89 @@ func TestWitnessUpdateSerialization(t *testing.T) { t.Fatalf("Serialize failed: %v", err) } - // The bug produced 132 bytes (extra 0x01 before props). Correct is 131. - if len(serialized) != 131 { - t.Errorf("expected 131 bytes, got %d bytes (hex: %s)", len(serialized), hex.EncodeToString(serialized)) + // Expected golden hex (131 bytes). Breakdown for reference: + // ec48 ref_block_num (18668 LE) + // f00f8b91 ref_block_prefix (2441809904 LE) + // 0924b6a0 expiration (unix 1783337488 LE) + // 01 op_count (1) + // 0b op_type (11 = witness_update) + // 06657479303031 owner "ety001" (varint-len 6) + // 1268...66616e73 url "https://steem.fans" (varint-len 18) + // 3553...4e716d block_signing_key (varint-len 53) + // b80b...000003 props.account_creation_fee (asset: uint64 3000, prec 3, "STEEM\0\0") + // 00010000 props.maximum_block_size (65536 LE) + // 0000 props.sbd_interest_rate (0 LE) + // 0000...0003 fee (asset: uint64 0, prec 3, "STEEM\0\0") + // 00 extensions count (0) + // + // CRITICAL: the byte after block_signing_key is 0xb8 (first byte of the + // asset amount), NOT 0x01. The old encoder inserted an erroneous 0x01 here. + const expectedHex = "ec48f00f8b9110924b6a010b066574793030311268747470733a2f2f737465656d2e66616e733553544d37634b6b7942437854526b64786f47535035797061674e656156596552376f576342734e664b75725867356a6e41744e716db80b00000000000003535445454d0000000001000000000000000000000003535445454d000000" + + got := hex.EncodeToString(serialized) + if got != expectedHex { + t.Errorf("golden hex mismatch:\n expected (%d bytes): %s\n got (%d bytes): %s", + len(expectedHex)/2, expectedHex, len(got)/2, got) + } +} + +// TestAccountUpdateOptionalFields verifies that truly optional pointer fields +// (account_update owner/active/posting) still emit the 0x01/0x00 presence byte, +// while non-optional pointers (witness_update props) do not. +func TestAccountUpdateOptionalFields(t *testing.T) { + expiration := time.Date(2026, 7, 6, 11, 31, 28, 0, time.UTC) + + activeAuth := &protocol.Authority{ + WeightThreshold: 1, + KeyAuths: protocol.StringInt64Map{"STM6z4ueBGtT96KczqAEzQQTxPUkX5FTVKMEjRb1viFeS2eBSGgKu": 1}, } - // The byte right after block_signing_key should NOT be 0x01 (pointer-present flag). - // It should be the first byte of the ChainProperties.account_creation_fee asset (0xb8). - keyStr := "STM7cKkyBCxTRkdxoGSP5ypagNeaVYeR7oWcBsNfKurXg5jnAtNqm" - keyEnd := -1 - for i := 0; i < len(serialized)-len(keyStr); i++ { - if string(serialized[i:i+len(keyStr)]) == keyStr { - keyEnd = i + len(keyStr) + // owner=nil (optional absent), active=set (optional present), posting=nil + op := &protocol.AccountUpdateOperation{ + Account: "ety001", + Owner: nil, // optional → should serialize as 0x00 + Active: activeAuth, + Posting: nil, // optional → should serialize as 0x00 + MemoKey: "STM6z4ueBGtT96KczqAEzQQTxPUkX5FTVKMEjRb1viFeS2eBSGgKu", + JsonMetadata: "", + } + + stx := NewSignedTransaction(&Transaction{ + RefBlockNum: 18668, + RefBlockPrefix: 2441809904, + Expiration: &protocol.Time{Time: &expiration}, + Extensions: []interface{}{}, + }) + stx.PushOperation(op) + + serialized, err := stx.Serialize() + if err != nil { + t.Fatalf("Serialize failed: %v", err) + } + + // Locate the account name "ety001" (varint-len 6 + "ety001") = 7 bytes after op_type. + // Layout: header(11) + op_count(1) + op_type(1) + acct_len(1) + "ety001"(6) + // = offset 20 → owner flag at offset 20. + acctEnd := -1 + acct := []byte("ety001") + for i := 0; i < len(serialized)-len(acct); i++ { + if string(serialized[i:i+len(acct)]) == "ety001" { + acctEnd = i + len(acct) break } } - if keyEnd == -1 { - t.Fatalf("could not find block_signing_key in serialized output") + if acctEnd == -1 { + t.Fatalf("could not find account name in serialized output") + } + + // owner (optional nil) → must be 0x00 + if serialized[acctEnd] != 0x00 { + t.Errorf("owner (optional, nil) should encode as 0x00, got 0x%02x", serialized[acctEnd]) } - if serialized[keyEnd] == 0x01 { - t.Errorf("found erroneous 0x01 pointer-present flag at offset %d; "+ - "witness_update props must serialize as plain struct, not optional", keyEnd) + // active (optional, set) → must be 0x01 followed by the Authority struct + activeOffset := acctEnd + 1 + if serialized[activeOffset] != 0x01 { + t.Errorf("active (optional, set) should encode as 0x01, got 0x%02x", serialized[activeOffset]) } }