Skip to content

fix: preserve outpoints list validation errors#5

Open
JasonZhouPW wants to merge 1 commit into
BitgesellOfficial:masterfrom
JasonZhouPW:fix/outpoints-list-errors
Open

fix: preserve outpoints list validation errors#5
JasonZhouPW wants to merge 1 commit into
BitgesellOfficial:masterfrom
JasonZhouPW:fix/outpoints-list-errors

Conversation

@JasonZhouPW

Copy link
Copy Markdown

Summary

  • preserves parameter validation errors for /rest/outpoints instead of converting them to JSON decode errors
  • rejects non-list JSON bodies separately from malformed outpoint entries
  • adds focused unit coverage for limits, invalid outpoints, non-list input, and valid parsing

Why

The handler previously parsed JSON and validated the list inside one broad except block. As a result, requests over the 100-outpoint limit or malformed entries were reported as JSON_DECODE_ERROR, hiding the actual parameter problem from API clients.

Validation

  • RED before fix: python3 -m pytest test/unit/test_outpoints_list.py -q failed because _parse_outpoints_list did not exist and validation was not isolated
  • GREEN after fix: python3 -m pytest test/unit/test_outpoints_list.py -q -> 4 passed
  • PYTHONPYCACHEPREFIX=/private/tmp/pycache python3 -m py_compile api/views/outpoints.py test/unit/test_outpoints_list.py
  • git diff --check

Bounty context: submitted for the Bitgesell improvement / PR bounty program, related to BitgesellOfficial/bitgesell#81 and BitgesellOfficial/bitgesell#32. Payout details can be provided after maintainer approval.

@MyTH-zyxeon

Copy link
Copy Markdown

Review-assist for #5 under the Bitgesell improvement queue (#81):

  • The split into _parse_outpoints_list() is a useful direction: it keeps JSON decoding failures separate from semantic outpoint validation, and the new unit tests make the 100-outpoint and malformed-entry cases much easier to review.
  • Please confirm the API contract for a valid JSON object such as {"outpoint": "..."} returning JSON_DECODE_ERROR rather than PARAMETER_ERROR. The helper now makes that behavior explicit, so it is worth locking in intentionally.
  • The new negative-index rejection is good, but I would add coverage for index 0 and a larger valid index to prove the serialized tx_id + int_to_bytes(index) key still matches the real pybgl.int_to_bytes() behavior, not only the local test stub.
  • The helper-level tests are focused, but one endpoint-level regression would make this safer: malformed JSON should still be reported as JSON_DECODE_ERROR, while over-limit and malformed outpoint arrays should escape the request-json except block as PARAMETER_ERROR.
  • Because this touches REST error semantics, a short manual/API smoke note for /rest/outpoints with non-list JSON, too many entries, one invalid outpoint, and one valid outpoint would make the maintainer acceptance path clear.
  • For #81 payout hygiene, this looks like a concrete API correctness/testability cleanup; I would still wait for maintainer confirmation before any claim or payment step.

I only reviewed the public diff. No duplicate implementation PR, secret material, dependency install, live node call, local checkout mutation, payment action, or live-chain action from me here.

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