Skip to content

docs: clarify RPC behavior and validate rustdoc - #273

Open
ayaanoncrypto wants to merge 3 commits into
circlefin:mainfrom
ayaanoncrypto:fix/docs-and-rustdoc
Open

docs: clarify RPC behavior and validate rustdoc#273
ayaanoncrypto wants to merge 3 commits into
circlefin:mainfrom
ayaanoncrypto:fix/docs-and-rustdoc

Conversation

@ayaanoncrypto

@ayaanoncrypto ayaanoncrypto commented Aug 19, 2026

Copy link
Copy Markdown

Closes #271
Closes #270
Closes #257
Closes #235

Summary

This pull request groups four focused documentation and CI improvements:

Validation

git diff --check passed. Rust formatting and rustdoc execution were unavailable locally because Cargo is not installed in the sandbox.

@osr21

osr21 commented Aug 19, 2026

Copy link
Copy Markdown

Reviewed the full diff against the five issues this closes (#235, #257, #270, #271, #272). Three of the five are cleanly resolved; two have gaps worth fixing before merge, and there's one CI ordering problem that will break the build.

Verified good:

Gaps:

1. The new rust-doc CI job will fail on its first run — this is the blocker. Issue #257 (which this PR closes) states in its own title that "31 rustdoc diagnostics have accumulated." This PR adds the enforcement (cargo doc --workspace --no-deps --all-features --locked with RUSTDOCFLAGS: -D warnings) but does not fix those 31 diagnostics. With -D warnings, the job turns every one of those into an error, so the check goes red immediately and blocks this PR and every subsequent one. The enforcement and the cleanup have to land together: either fix the 31 rustdoc warnings in this PR (or a predecessor), or land the job first without -D warnings (report-only), clean up, then flip to -D warnings. As-is, #257 isn't actually closed — the diagnostics still exist, they're just now fatal. (I can't run cargo doc here — no Rust toolchain in my sandbox — so I'm taking the count from #257's own claim; the ordering problem holds for any non-zero count.)

2. #272 — the prose is fixed but the printf bug in the same two lines is not. The diff correctly changes "next block height" → "latest block number known to the node", but keeps printf "%0d":

It is not the next block height. You can use `printf "%0d"` to translate it into decimal.

%0d is not a valid hex→decimal conversion — %d doesn't parse a 0x-prefixed string and 0 alone isn't a valid flag/width, so printf "%0d" 0x64 does not yield 100. The correct form is:

printf "%d\n" 0x64   # -> 100

Since this PR closes #272 and edits these exact two lines, it's the natural place to fix the snippet too.

3. #270 is only partially addressed. The [RPC] flag is defined and applied to the gascap entry, but the other externally-observable RPC changes already in BREAKING_CHANGES.md are left tagged operator-only:

  • v0.7.2 pre-EIP-155 rejection (currently [CLI]) — clients submitting legacy unprotected txs break; that's app-facing → should be [CLI][RPC].
  • v0.7.2 batch cap (--arc.rpc.max-batch-entries, currently [Config]) — oversized batches now rejected with -32600; app-facing → [Config][RPC].
  • v0.7.1 subscription cap (--rpc.max-subscriptions-per-connection 1024→32, currently [Config]) — WS clients opening >32 subs break; app-facing → [Config][RPC].

Closing #270 while only tagging one of the four known app-facing entries leaves the taxonomy half-applied. Either annotate these three here, or scope #270 to "introduce the category" and track the retro-annotation separately.

Minor (CI supply-chain consistency): actions/checkout is pinned to a commit SHA (df4cb1c… # v6.0.3), but actions-rust-lang/setup-rust-toolchain@v1 in the same new job uses a floating major tag. For consistency with the pinned checkout, pin setup-rust-toolchain to a SHA as well.

Net: #235/#271 are done; #270/#272 need small follow-through in this same PR; and the rust-doc job needs the pre-existing warnings resolved (or a report-only first step) or CI breaks the moment this lands. Happy to help with the rustdoc cleanup or the BREAKING_CHANGES.md annotations if useful.

@osr21

osr21 commented Aug 19, 2026

Copy link
Copy Markdown

Thanks for the quick turnaround — the follow-up commit (573ce38) resolves three of the four items:

Still outstanding — and it's the merge blocker: the rust-doc job (#257). This commit doesn't touch it. The job still runs cargo doc … with RUSTDOCFLAGS: -D warnings, and the 31 pre-existing rustdoc diagnostics that #257 names haven't been fixed, so the check will fail on its first run and block the merge. This needs one of:

  1. fix the 31 rustdoc warnings in this PR (then -D warnings passes and docs: cargo doc is not run in CI; 31 rustdoc diagnostics have accumulated #257 is genuinely closed), or
  2. land the job report-only first (drop -D warnings), clean up the diagnostics in a follow-up, then flip enforcement on.

Once CI is green on the rust-doc job this looks good to me. (Still can't run cargo doc locally — no Rust toolchain in my sandbox — so I'm going off #257's stated count; the ordering constraint holds for any non-zero number.)

@osr21

osr21 commented Aug 20, 2026

Copy link
Copy Markdown

Re: dee374b ("docs: deduplicate block number clarification") — I see what you did here, and the dedup mechanic is right, but there's one loose end that will bite on merge.

What this commit does: it reverts #273's copy of the eth_blockNumber doc fix back to the original text, so this file now matches main exactly (blob 93f04f9). Net effect: #273 no longer touches docs/running-an-arc-node.md, which cleanly avoids the conflict with #274 and lets #274 own the #272 fix. Good — that's exactly the deduplication.

The problem: the reverted text is the buggy original — "next block height" and printf "%0d", the two things #272 reported. That's harmless only because it now equals main… but #273's description still says Closes #272. So if #273 merges, GitHub auto-closes #272 while shipping the un-fixed (buggy) text, and the real fix would then live only in #274. If #274 hasn't merged yet at that point, #272 is closed with the bug still live in main.

Two things to fix before this merges:

  1. Remove #272 from docs: clarify RPC behavior and validate rustdoc #273's closing list — leave it as Closes #235, #257, #270, #271. docs: clarify RPC behavior and validate rustdoc #273 no longer fixes docs: node verification guide incorrectly describes eth_blockNumber as returning the next block height #272, so it shouldn't close it.
  2. Land docs: clarify eth_blockNumber verification #274 to actually fix docs: node verification guide incorrectly describes eth_blockNumber as returning the next block height #272. docs: clarify eth_blockNumber verification #274 (head 2e6a4e9) still carries the correct text ("latest block number known to the node" + printf "%d\n" <hex>) and is independent of the rust-doc/docs: cargo doc is not run in CI; 31 rustdoc diagnostics have accumulated #257 CI work still blocking docs: clarify RPC behavior and validate rustdoc #273 — so it can go in first and close docs: node verification guide incorrectly describes eth_blockNumber as returning the next block height #272 on its own.

After that the split is clean: #274#272; #273#235/#257/#270/#271 (still pending the rust-doc job resolving those 31 rustdoc diagnostics so -D warnings passes).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants