Skip to content

rpc: fix SendRawTransaction_Result.Reason json tag, add tag-hygiene tests - #75

Open
DHEBP wants to merge 1 commit into
DEROFDN:community-devfrom
DHEBP:fix/rpc-json-tag-lint
Open

rpc: fix SendRawTransaction_Result.Reason json tag, add tag-hygiene tests#75
DHEBP wants to merge 1 commit into
DEROFDN:community-devfrom
DHEBP:fix/rpc-json-tag-lint

Conversation

@DHEBP

@DHEBP DHEBP commented Aug 15, 2026

Copy link
Copy Markdown

SendRawTransaction_Result.Reason is tagged json:"string" instead of
json:"reason", so every DERO.SendRawTransaction response carries an
undocumented "string" key and a client reading "reason" finds nothing.

Impact today is nil rather than data loss. Reason is never assigned anywhere in
the tree, so the field is always empty. The two soft-failure paths in
rpc_dero_sendrawtransaction.go report via result.Status, and real rejections
return a Go error that becomes a JSON-RPC error object, so the failure text a
client actually sees is unaffected either way.

I corrected the tag rather than removing the field. Whether to populate it or
delete it is an API decision for maintainers, and populating it would mean
changing the rejection branch to return a result instead of an error -- that
removes the JSON-RPC error object which is currently the only rejection signal
derohe's own wallet reads, so it is a breaking change and should not ride along
with a tag fix.

The second half is a guard so this class does not recur. rpc/jsontag_test.go
parses the package source (rather than using reflection) and asserts:

  • no json tag named after a Go builtin type -- this is what the defect above
    looks like mechanically: the field's TYPE written where its NAME belongs
  • no duplicate json tag within a struct, where one field silently never
    reaches the wire and which one wins depends on field order
  • json tags are lowercase, matching the rest of the wire format

It runs under go test ./rpc/. No new dependency and no CI job required.

Verification: the first check fails on the unfixed tree, naming the exact
file:line and suggesting the correct tag, and passes after the fix. The other
two pass in both states, so the suite is not merely failing everything. I
re-confirmed this after rebasing by reintroducing the bad tag and watching the
check fire. go build ./... is clean and go test ./rpc/ passes.

Every type in this package is part of derod's or the wallet's public wire
format, and a wrong tag there is invisible at compile time and invisible to
tests that only exercise Go structs -- it only shows up as a field a client can
never read.

…ests

SendRawTransaction_Result.Reason was tagged `json:"string"` instead of
`json:"reason"`, so every DERO.SendRawTransaction response carries an
undocumented "string" key and a client reading "reason" finds nothing.

Impact today is nil rather than data loss: Reason is never assigned anywhere in
the tree, so the field is always "". The two soft-failure paths in
rpc_dero_sendrawtransaction.go report via result.Status, and real rejections
return a Go error that becomes a JSON-RPC error object, so the failure text a
client sees is unaffected either way. The tag is corrected rather than the field
removed; whether to populate it or delete it is an API decision for maintainers,
and populating it would mean changing the rejection branch to return a result
instead of an error, which IS a breaking change and should not ride along here.

rpc/jsontag_test.go adds three checks over the whole package, parsing the source
so no one has to remember to register a type:

  - no json tag named after a Go builtin type (this is what the above defect
    looks like mechanically: the field's TYPE written where its NAME belongs)
  - no duplicate json tag within a struct, where one field silently never
    reaches the wire and the winner depends on field order
  - json tags are lowercase, matching the rest of the wire format

These run with `go test ./rpc/` -- no new dependency, no CI job required. The
first check fails on the unfixed tree and passes after, while the other two pass
in both states, so the suite is not merely failing everything.
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.

1 participant