Skip to content

Commit f770b8a

Browse files
ety001ety001
committed
Fix custom_json operation serialization and improve test coverage
- Fix CustomJSONOperation.MarshalTransaction to include operation type code This was causing "missing required posting authority" errors when broadcasting custom_json operations. The operation type code (18 = 0x12) must be encoded at the beginning of the operation serialization. - Add sorting for required_auths and required_posting_auths fields These fields are flat_set types in Steem C++ and must be sorted before serialization to match blockchain expectations. - Add comprehensive unit tests for CustomJSONOperation - TestMarshalTransaction: Tests direct MarshalTransaction calls with type code - TestFullOperationEncoding: Tests complete operation encoding via encoder.Encode() - TestMarshalTransaction_Sorting: Verifies flat_set field sorting All tests verify compatibility with steem-js serialization. - Add debug output for JSON-RPC requests when DEBUG env var is set Helps diagnose transaction broadcasting issues. - Improve Time.UnmarshalJSON to handle quoted and unquoted time strings More robust parsing for ISO8601 time format. - Add documentation explaining the fix and why unit tests didn't catch it - custom_json_operation_type_code_fix.md: Detailed analysis of the bug - signature_format_analysis.md: Signature format verification - transaction_serialization_verification.md: Transaction serialization details
1 parent 3691182 commit f770b8a

9 files changed

+1076
-2
lines changed

‎docs/custom_json_operation_type_code_fix.md‎

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
# CustomJSONOperation Operation Type Code Fix
2+
3+
## Problem Summary
4+
5+
The `CustomJSONOperation.MarshalTransaction` method was missing the operation type code encoding, causing transactions to be serialized incorrectly. This document explains why this bug wasn't caught earlier.
6+
7+
## Why Other Operations Worked
8+
9+
### Encoder Logic Flow
10+
11+
The `encoder.Encode()` method follows this priority:
12+
13+
1. **Check for `MarshalTransaction` method** (line 50):
14+
```go
15+
if marshaller, ok := v.(TransactionMarshaller); ok {
16+
return marshaller.MarshalTransaction(encoder)
17+
}
18+
```
19+
20+
2. **Check if it's an Operation interface** (line 55):
21+
```go
22+
if op := encoder.checkOperation(v); op != nil {
23+
return encoder.encodeOperation(op) // Automatically encodes type code
24+
}
25+
```
26+
27+
### Two Paths for Operations
28+
29+
**Path 1: Operations WITH `MarshalTransaction` method**
30+
- `VoteOperation`, `CommentOperation`, `CustomJSONOperation` (before fix)
31+
- These methods are called directly
32+
- **They must manually encode the operation type code**
33+
- Example from `VoteOperation`:
34+
```go
35+
func (op *VoteOperation) MarshalTransaction(encoderObj *encoder.Encoder) error {
36+
enc.EncodeUVarint(uint64(TypeVote.Code())) // ✅ Encodes type code
37+
enc.Encode(op.Voter)
38+
// ...
39+
}
40+
```
41+
42+
**Path 2: Operations WITHOUT `MarshalTransaction` method**
43+
- Operations that don't implement `MarshalTransaction`
44+
- Go through `encodeOperation()` which **automatically encodes the type code**:
45+
```go
46+
func (encoder *Encoder) encodeOperation(op *operationInterface) error {
47+
// Encode the operation type code
48+
code := op.getTypeCode()
49+
encoder.EncodeUVarint(code) // ✅ Automatically encoded
50+
// Then encode operation data
51+
return encoder.encodeByReflection(opData)
52+
}
53+
```
54+
55+
### Why CustomJSONOperation Failed
56+
57+
`CustomJSONOperation` had a `MarshalTransaction` method but **didn't encode the type code**:
58+
59+
```go
60+
// BEFORE FIX (WRONG)
61+
func (op *CustomJSONOperation) MarshalTransaction(encoderObj *encoder.Encoder) error {
62+
// ❌ Missing: encoderObj.EncodeUVarint(uint64(op.Type().Code()))
63+
encoderObj.EncodeUVarint(uint64(len(requiredAuths)))
64+
// ... rest of encoding
65+
}
66+
```
67+
68+
This caused the operation type code (18 for `custom_json`) to be missing, making the blockchain interpret it as operation type 0 (`vote`), which would fail validation.
69+
70+
## Why Unit Tests Didn't Catch This
71+
72+
### Test Implementation
73+
74+
The unit test in `operation_custom_json_test.go` directly calls `MarshalTransaction`:
75+
76+
```go
77+
err := tt.op.MarshalTransaction(enc) // Direct call, not through encoder.Encode()
78+
```
79+
80+
### What the Test Validated
81+
82+
The test compared the output of `MarshalTransaction` with expected hex values from `old-steem-js`:
83+
84+
```go
85+
expectedHex2 := "000105616c69636506666f6c6c6f77315b22666f6c6c6f77222c7b22666f6c6c6f776572223a22616c696365222c22666f6c6c6f77696e67223a22626f62227d5d"
86+
```
87+
88+
### Why It Matched
89+
90+
The expected hex from `old-steem-js` was generated using:
91+
92+
```javascript
93+
const buf = custom_json.toBuffer(testCase.op); // Only operation DATA, not full operation
94+
```
95+
96+
This `custom_json.toBuffer()` method in steem-js **only serializes the operation data**, not the full operation tuple `[type_code, data]`. It's equivalent to Go's `MarshalTransaction` method.
97+
98+
So the test was correctly validating that the **operation data** serialization matched, but it wasn't testing the **full operation** serialization (which includes the type code).
99+
100+
### The Missing Test
101+
102+
The test should have also tested the full operation encoding path:
103+
104+
```go
105+
// Test full operation encoding (what actually happens in transactions)
106+
err := enc.Encode(tt.op) // Goes through encoder.Encode() → encodeOperation()
107+
```
108+
109+
This would have caught the missing type code.
110+
111+
## Fix
112+
113+
Added operation type code encoding to `CustomJSONOperation.MarshalTransaction`:
114+
115+
```go
116+
// AFTER FIX (CORRECT)
117+
func (op *CustomJSONOperation) MarshalTransaction(encoderObj *encoder.Encoder) error {
118+
// ✅ Encode operation type code first
119+
if err := encoderObj.EncodeUVarint(uint64(op.Type().Code())); err != nil {
120+
return errors.Wrap(err, "failed to encode operation type code")
121+
}
122+
// ... rest of encoding
123+
}
124+
```
125+
126+
## Lessons Learned
127+
128+
1. **Operations with `MarshalTransaction` must encode type code manually**
129+
2. **Unit tests should test both paths**:
130+
- Direct `MarshalTransaction` call (operation data only)
131+
- Through `encoder.Encode()` (full operation with type code)
132+
3. **When comparing with steem-js**, ensure you're comparing the same level:
133+
- `custom_json.toBuffer()` = operation data only
134+
- `operation.toBuffer([type, data])` = full operation with type code
135+

‎docs/signature_format_analysis.md‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
# Signature Format Analysis
2+
3+
## Summary
4+
5+
After analyzing the signature format used by `btcec.SignCompact` and comparing it with `steem-js` and Steem C++ implementations, we found:
6+
7+
### Key Findings
8+
9+
1. **Recovery Parameter Format**: ✅ Correct
10+
- `btcec.SignCompact` returns recovery ID in format: `27 + 4 + recovery_param` for compressed keys (31-34)
11+
- This matches Steem's expected format
12+
- Steem C++ extracts recovery param as: `(recoveryID - 27) & 3`
13+
14+
2. **Canonical Signature**: ✅ Correct
15+
- `btcec.SignCompact` automatically generates canonical signatures
16+
- Test confirms signatures pass `is_fc_canonical` check:
17+
- `!(c.data[1] & 0x80)` - r[0] doesn't have high bit set
18+
- `!(c.data[1] == 0 && !(c.data[2] & 0x80))` - if r[0] == 0, r[1] must have high bit set
19+
- `!(c.data[33] & 0x80)` - s[0] doesn't have high bit set
20+
- `!(c.data[33] == 0 && !(c.data[34] & 0x80))` - if s[0] == 0, s[1] must have high bit set
21+
22+
3. **Public Key Recovery**: ✅ Correct
23+
- Recovered public key matches expected public key
24+
- Compression flag is correctly set
25+
26+
### Comparison with steem-js
27+
28+
**old-steem-js** (signature.js:88-90):
29+
```javascript
30+
i = ecdsa.calcPubKeyRecoveryParam(curve, e, ecsignature, private_key.toPublicKey().Q);
31+
i += 4; // compressed
32+
i += 27; // compact
33+
// Final: 27 + 4 + (0-3) = 31-34
34+
```
35+
36+
**steem-js** (signature.ts:88-89):
37+
```typescript
38+
const i = calcPubKeyRecoveryParam(secp256k1, new BN(buf_sha256), ecsignature, privKey.toPublic().Q!);
39+
return new Signature(ecsignature.r, ecsignature.s, i + 27);
40+
// Note: toCompact() method adds 4 for compressed keys
41+
```
42+
43+
**Go (steemutil)**:
44+
```go
45+
sig, err := ecdsa.SignCompact(privKey.Raw.PrivKey, digest, true)
46+
// Returns recovery ID in format: 27 + 4 + recovery_param (31-34) for compressed keys
47+
```
48+
49+
### Steem C++ Implementation
50+
51+
**elliptic_secp256k1.cpp:164**:
52+
```cpp
53+
secp256k1_ecdsa_recover_compact(
54+
detail::_get_context(),
55+
(unsigned char*) digest.data(),
56+
(unsigned char*) c.begin() + 1, // Skip recovery ID byte
57+
(unsigned char*) my->_key.begin(),
58+
(int*) &pk_len,
59+
1, // compressed flag
60+
(*c.begin() - 27) & 3 // Extract recovery param: (recoveryID - 27) & 3
61+
);
62+
```
63+
64+
**elliptic_impl_pub.cpp:343-348**:
65+
```cpp
66+
if (nV >= 31) {
67+
EC_KEY_set_conv_form(my->_key, POINT_CONVERSION_COMPRESSED);
68+
nV -= 4; // Remove compressed flag
69+
}
70+
```
71+
72+
### Conclusion
73+
74+
The `btcec.SignCompact` implementation is **correct** and matches Steem's expected format. The signature format, recovery parameter encoding, and canonical signature requirements are all properly handled.
75+
76+
If signature verification fails on the blockchain, the issue is likely:
77+
1. **Not related to signature format** - the format is correct
78+
2. **May be related to transaction serialization** - ensure transaction bytes match exactly
79+
3. **May be related to digest calculation** - ensure chain ID and transaction bytes are concatenated correctly
80+
4. **May be related to key matching** - ensure the private key matches the account's posting key
81+
82+
### Test Results
83+
84+
```
85+
✅ Recovery ID 32 is in expected range for compressed keys (31-34)
86+
✅ Signature is canonical (fc_canonical format)
87+
✅ Recovered public key matches!
88+
```
89+

‎docs/transaction_serialization_verification.md‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
# Transaction Serialization and Digest Calculation Verification
2+
3+
## Summary
4+
5+
After thorough analysis and testing, we have verified that:
6+
7+
### ✅ Transaction Serialization: Correct
8+
9+
1. **Signatures are excluded**: Transaction serialization correctly excludes signatures when calculating the digest
10+
- Test: `TestTransactionSerializationExcludesSignatures` confirms that transactions with and without signatures serialize to the same bytes
11+
- This matches Steem C++ behavior where `transaction::sig_digest()` serializes the `transaction` object (not `signed_transaction`)
12+
13+
2. **Serialization format**: The transaction serialization format matches Steem protocol:
14+
- `ref_block_num` (uint16)
15+
- `ref_block_prefix` (uint32)
16+
- `expiration` (time_point_sec - uint32)
17+
- `operations` (varint length + operations)
18+
- `extensions` (varint length + extensions)
19+
- **No signatures** in digest calculation
20+
21+
### ✅ Digest Calculation: Correct
22+
23+
1. **Chain ID format**: Chain ID is correctly decoded from hex string (64 hex chars = 32 bytes)
24+
- Steem C++: `fc::sha256` (32 bytes) serialized via `fc::raw::pack`
25+
- Go: `hex.DecodeString(chain.ID)` produces 32 bytes ✅
26+
27+
2. **Digest formula**: `sha256(chain_id + serialized_transaction)`
28+
- Steem C++: `fc::raw::pack(enc, chain_id); fc::raw::pack(enc, *this); return enc.result();`
29+
- Go: `msgBuffer.Write(rawChainID); msgBuffer.Write(rawTx); sha256.Sum256(msgBuffer.Bytes())` ✅
30+
31+
3. **Test verification**: `TestTransaction_Digest` passes with expected digest value
32+
33+
### Comparison with steem-js
34+
35+
**steem-js** (auth/index.ts:136-141):
36+
```typescript
37+
const chainId = (getConfig().get('chain_id') as string | undefined) || '';
38+
const cid = Buffer.from(chainId, 'hex');
39+
const buf = transaction.toBuffer(trx as unknown);
40+
const sig = Signature.signBuffer(Buffer.concat([cid, buf]), key);
41+
```
42+
43+
**Go (steemutil)**:
44+
```go
45+
rawChainID, err := hex.DecodeString(chain.ID)
46+
msgBuffer.Write(rawChainID)
47+
rawTx, err := tx.Serialize()
48+
msgBuffer.Write(rawTx)
49+
digest := sha256.Sum256(msgBuffer.Bytes())
50+
```
51+
52+
Both implementations:
53+
1. Decode chain ID from hex string to bytes
54+
2. Serialize transaction (without signatures)
55+
3. Concatenate chain_id + transaction
56+
4. Calculate SHA256 digest
57+
58+
### Test Results
59+
60+
```
61+
✅ Transaction serialization correctly excludes signatures
62+
✅ Digests match (signatures correctly excluded)
63+
✅ Digest calculation matches expected format
64+
```
65+
66+
### Conclusion
67+
68+
The transaction serialization and digest calculation are **correct** and match both Steem C++ and steem-js implementations. The issue with signature verification on the blockchain is **not** related to:
69+
- Transaction serialization format
70+
- Digest calculation method
71+
- Chain ID encoding
72+
73+
The problem is likely related to:
74+
1. **Signature recovery**: The signature format is correct, but the blockchain may be using a different recovery method
75+
2. **Key matching**: The private key may not match the account's posting key (though we've verified this)
76+
3. **Transaction preparation**: The ref_block_num and ref_block_prefix may need to be set correctly before signing
77+
78+
### Next Steps
79+
80+
To debug further, we should:
81+
1. Compare the actual signed transaction bytes with steem-js output
82+
2. Verify the signature recovery process on the blockchain side
83+
3. Check if there are any differences in how the blockchain validates signatures
84+

‎jsonrpc2/jsonrpc2.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ package jsonrpc2
33
import (
44
"bytes"
55
"encoding/json"
6+
"fmt"
67
"net/http"
8+
"os"
79
"time"
810

911
"github.com/pkg/errors"
@@ -31,6 +33,12 @@ func (j *JsonRpc) BuildSendData(method string, params []any) (err error) {
3133
return
3234
}
3335
j.SendData = tmp
36+
37+
// Debug: Print JSON request if DEBUG is set
38+
if os.Getenv("DEBUG") != "" && method == "condenser_api.broadcast_transaction_synchronous" {
39+
fmt.Printf("=== JSON-RPC Request ===\n%s\n", string(tmp))
40+
}
41+
3442
return
3543
}
3644

0 commit comments

Comments
 (0)