fix(tm2): Fix the endpoint /tx?hash= to make it work #2518
Conversation
linhpn99
requested review from
jaekwon,
moul,
piux2 and
zivkovicmilos
as code owners
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2518 +/- ##
==========================================
+ Coverage 60.44% 60.45% +0.01%
==========================================
Files 563 563
Lines 75159 75092 -67
==========================================
- Hits 45427 45398 -29
+ Misses 26343 26309 -34
+ Partials 3389 3385 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
linhpn99
changed the title
| // Handle int string | ||
| if reInt.Match([]byte(arg)) { | ||
| arg = fmt.Sprintf("%q", arg) |
There was a problem hiding this comment.
Why is this necessary? I think things like height= values will be parsed incorrectly if we have this...
There was a problem hiding this comment.
No, I only quote in the case of integers. You need to format it as a JSON string by quoting it; otherwise, you will encounter an error when calling UnmarshalJSON
There was a problem hiding this comment.
If the data is a raw number (e.g., -10) and you pass it directly as json.RawMessage, it will be interpreted as a JSON number, not as a string
| // Handle base64 string | ||
| if decoded, err := base64.StdEncoding.DecodeString(arg); err == nil { | ||
| data, err := amino.MarshalJSON(decoded) | ||
| if err != nil { | ||
| return nil, errors.Wrap(err, "error marshaling argument to JSON") | ||
| } |
There was a problem hiding this comment.
Please remove this
- many strings will be decodable as base64 when they shouldn't be.
- for /tx?hash= specifically, it will work; the decoding will be handled by
amino.UnmarshalJSONin mapParamsToArgs, as long as the string is quoted here
linhpn99
force-pushed
the
fix-query-tx-by-hash
branch
from
c9fe0c8
to
b4b9b2d
Compare
From issue : #2489
I found an issue with the code segment parsing arguments from the request that did not support base64 encoding, so I implemented logic to handle it. If there are any inconsistencies, please share the opinion
Contributors' checklist...
BREAKING CHANGE: xxxmessage was included in the description