Skip to content

Fix block headers URL when count is omitted - #7

Open
ColumbusLabs wants to merge 1 commit into
BitgesellOfficial:mainfrom
ColumbusLabs:codex/bitgesell-blockheaders-optional-count
Open

Fix block headers URL when count is omitted#7
ColumbusLabs wants to merge 1 commit into
BitgesellOfficial:mainfrom
ColumbusLabs:codex/bitgesell-blockheaders-optional-count

Conversation

@ColumbusLabs

Copy link
Copy Markdown

Summary

  • Fix getBlockHeaders() so omitting the optional count argument calls /block/headers/{height} instead of /block/headers/{height}/undefined.
  • Preserve the counted form so getBlockHeaders(10, 2) still calls /block/headers/10/2.
  • Add a mocked URL regression test covering both request shapes.

Evidence

  • BitgesellOfficial/bglapiserver registers both /rest/block/headers/{block_pointer} and /rest/block/headers/{block_pointer}/{count}.
  • The bglapiserver API docs mark count as not required for block headers.

Validation

  • npm ci --legacy-peer-deps
  • npm run build
  • npm test -- --runTestsByPath src/__tests__/blockchain-url.test.ts --coverage=false
  • git diff --check

Submitted for the Bitgesell bounty/improvement program: BitgesellOfficial/bitgesell#81. Payment details can be provided after review/approval.

@MyTH-zyxeon

Copy link
Copy Markdown

Maintainer review-assist for BitgesellOfficial/bitgesell#81:

This patch looks directionally correct and stays narrow: it removes the /undefined suffix when count is omitted, preserves the counted route, and adds a focused regression test for both request shapes.

Suggested acceptance checks:

  1. Confirm the SDK route now matches the two visible bglapiserver shapes: /rest/block/headers/{block_pointer} and /rest/block/headers/{block_pointer}/{count}.
  2. Verify the mocked regression test is enough for this bug class, since the failure mode is URL construction rather than response parsing.
  3. Check whether any adjacent optional-path helpers in this SDK still interpolate undefined into REST paths, so the maintainer can batch similar route fixes if needed.
  4. Preserve the scope as a route-construction fix plus regression coverage only; no broader API behavior change seems necessary from the visible diff.

If those checks pass, this looks like a clean candidate for the #81 review queue.

@ColumbusLabs

Copy link
Copy Markdown
Author

Hi maintainers — this focused URL-construction fix is still current with main and includes regression coverage for both the counted and omitted-count routes. Could someone take a look when convenient? I am happy to make any requested adjustments.

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