Skip to content

docs: clarify eth_blockNumber verification - #274

Open
ayaanoncrypto wants to merge 1 commit into
circlefin:mainfrom
ayaanoncrypto:docs/fix-eth-block-number-272
Open

docs: clarify eth_blockNumber verification#274
ayaanoncrypto wants to merge 1 commit into
circlefin:mainfrom
ayaanoncrypto:docs/fix-eth-block-number-272

Conversation

@ayaanoncrypto

Copy link
Copy Markdown

Addresses issue #272.

Correct the node verification guide’s interpretation of eth_blockNumber. The RPC result identifies the latest block known to the node, not the next block height. Also fix the decimal conversion example.

Validation:

  • Documentation text scan passed.
    • git diff --check passed.

@osr21

osr21 commented Aug 20, 2026

Copy link
Copy Markdown

The fix here is correct: result is the latest block number known to the node (not the next block height), and printf "%d\n" <hex> is the right hex→decimal conversion (bash printf %d parses a 0x-prefixed operand as hex). "Encoded as a hexadecimal quantity" also matches the JSON-RPC "quantity" wording. So on its own, this is a clean, accurate change.

The issue is duplication with #273 — please deduplicate before merging. #273 (also yours) declares Closes #272 and its latest commit (573ce38) already edits these exact two lines with an equivalent fix:

The result field represents the latest block number known to the node, in hexadecimal. It is not the next block height. You can use printf "%d\n" 0x64 to translate a hexadecimal value into decimal.

So both open PRs touch the same lines in docs/running-an-arc-node.md and both claim #272. Whichever merges second will hit a conflict and re-close an already-closed issue. Recommend picking one home for the #272 fix:

Either is fine — the point is that the same two lines shouldn't be fixed in two open PRs. My weak preference: land #274 for #272 and slim #273 down, since a 2-line docs PR is trivial to review and merge, and it unblocks #272 independently of the rust-doc/#257 CI work that's currently blocking #273.

@ayaanoncrypto

Copy link
Copy Markdown
Author

Follow-up to the deduplication review:

PR #273 no longer changes docs/running-an-arc-node.md and its description no longer closes #272. PR #274 remains the sole owner of the eth_blockNumber wording and printf "%d\n" <hex> fix for #272. PR #274 is mergeable with its checks complete and is ready for maintainer review.

Please review and merge #274 independently. This keeps #272 separate from the rustdoc enforcement work in #273 and avoids a documentation conflict.

@osr21

osr21 commented Aug 20, 2026

Copy link
Copy Markdown

Confirmed the dedup landed correctly, and #274 is the clean owner of the #272 fix:

So the split is exactly right. One correction on "mergeable with checks complete," since it's worth setting maintainer expectations accurately:

#274's mergeable_state is currently blocked, not clean. Breaking down what I see on head 2e6a4e9:

  • No merge conflicts (mergeable=true).
  • Check-runs: 5 total, all completed — 1 success, 4 skipped (Release binaries + the Rust build/clippy/test jobs). The skips are expected and correct: those workflows are path-filtered and a docs-only diff doesn't trigger them.
  • Legacy combined commit-status is empty (total_count=0), so the "pending" you may see there is just the no-statuses default, not a running job.

The blocked state therefore isn't something in the PR you can fix — it's the maintainer-side gate: this repo's branch protection needs a required approving review (there are 0 reviews so far) and/or requires a status context that gets skipped on docs-only changes. If it's the latter, a maintainer with admin can either mark the skipped required check as satisfied for this PR or merge via override; if it's just the review requirement, one maintainer approval clears it. Either way, "ready for maintainer review" is fair — it just can't merge on green check-runs alone. Flagging so whoever picks this up knows the remaining step is on their side, not yours.

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