Skip to content

refactor: audit cleanup — remove dead code, fix stake bug, consolidate helpers#93

Merged
wcatz merged 4 commits intomasterfrom
audit/cleanup-dead-code-and-docs
Mar 8, 2026
Merged

refactor: audit cleanup — remove dead code, fix stake bug, consolidate helpers#93
wcatz merged 4 commits intomasterfrom
audit/cleanup-dead-code-and-docs

Conversation

@wcatz
Copy link
Copy Markdown
Owner

@wcatz wcatz commented Mar 8, 2026

Summary

Full audit cleanup pass. Net: -879 lines.

Dead Code Removed

  • GetNonceValuesForEpoch, StreamBlockNonces, GetLastBlockHashForEpoch — removed from Store interface, SqliteStore, PgStore, and all tests (test-only, zero production callers)
  • BlockNonceRows interface + sqliteBlockNonceRows/pgBlockNonceRows impls — orphaned when StreamBlockNonces was removed
  • fetchLastBlockHashFromKoios — NonceTracker method, zero callers, misleading comment implied it was used as TICKN fallback
  • IntegrityResult struct — never instantiated (IntegrityReport kept)
  • DBIntegrityResult.Valid and .Repaired fields — only Truncated is checked in main.go
  • QueryStakeDistribution — fetches all 3000+ pools via NtC nil query; useless for single-pool leaderlog (QueryPoolStakeSnapshots already covers that need correctly)
  • cmd/ directory — empty

Bug Fixed

Auto-triggered leaderlog Koios fallback was using [curEpoch, curEpoch-1, curEpoch-2] instead of [targetEpoch, curEpoch, curEpoch-1, curEpoch-2]. This meant the auto-triggered schedule (the one stored to DB and announced to Telegram) could silently use stake from the wrong epoch, while a manual /leaderlog re-request of the same epoch might get different values.

All three leaderlog stake-fetch paths (on-demand /leaderlog, auto-trigger at 60%, /nextblock) now share queryStakeForLeaderlog() which uses the correct epoch order.

Refactors

  • fetchDuckURL() extracted; getDuckMedia() and fetchDuckByType() now share it (were ~90% identical)
  • queryStakeForLeaderlog() replaces 3 duplicate NtC+Koios blocks in commands.go

Docs / Config

  • docs/twitter-setup.md rewritten — previously claimed image support not implemented; code has full GIF/JPEG media upload via chunked Twitter API
  • README.md multi-arch claim corrected (ARM64-only, not amd64/arm64)
  • leaderlog.nonceIntegrityCheck and leaderlog.backfillSchedules documented in config.yaml.example (both existed in code but were undiscoverable)
  • 5x fmt.Sprintf("" + koiosRestBase + ...) noise removed
  • Misleading nonce.go comment corrected (described abandoned code path)

Summary by CodeRabbit

  • Documentation

    • Simplified Twitter/X setup guide and clarified ARM64 image publishing in the README.
    • Added commented advanced configuration options for nonce integrity checks and schedule backfilling (opt-in).
  • Refactor

    • Consolidated stake retrieval logic used by leader-schedule calculations and duck media fetching, reducing duplication.
  • Breaking Changes

    • Removed nonce streaming and single-epoch nonce query APIs and associated tests; integrity results simplified to indicate truncation only.

…e helpers

Dead code removed:
- GetNonceValuesForEpoch, StreamBlockNonces, GetLastBlockHashForEpoch from Store
  interface, SqliteStore, PgStore, and their test functions
- BlockNonceRows interface + sqliteBlockNonceRows/pgBlockNonceRows impls (orphaned
  when StreamBlockNonces was removed)
- fetchLastBlockHashFromKoios (NonceTracker method, zero callers)
- IntegrityResult struct (nonce.go, never instantiated — IntegrityReport kept)
- DBIntegrityResult.Valid and .Repaired fields (only Truncated is checked in main.go)
- QueryStakeDistribution (fetches all 3000+ pools via NtC nil query, useless for
  single-pool leaderlog — QueryPoolStakeSnapshots already covers that need)
- cmd/ directory (empty)

Bug fixed:
- Auto-triggered leaderlog Koios fallback was missing targetEpoch from its epoch
  list, causing it to always use current-epoch stake instead of next-epoch stake.
  All three leaderlog stake-fetch paths now use queryStakeForLeaderlog() which
  correctly tries [targetEpoch, curEpoch, curEpoch-1, curEpoch-2] on Koios.

Refactors:
- Extract fetchDuckURL() helper; getDuckMedia() and fetchDuckByType() now share it
- Extract queryStakeForLeaderlog() helper; replaces 3 duplicate NtC+Koios blocks
  in cmdLeaderlog (on-demand), auto-trigger leaderlog, and cmdNextBlock
- Remove snapType variable from cmdLeaderlog (helper computes it internally)

Comments and formatting:
- Fix misleading nonce.go:397 comment (described abandoned GetLastBlockHashForEpoch
  fallback that was never implemented; now describes actual code path)
- Remove 5x fmt.Sprintf("" + koiosRestBase + ...) noise in main.go and nonce.go

Documentation:
- Rewrite docs/twitter-setup.md (previously claimed image support not implemented;
  code has full GIF/JPEG media upload via chunked Twitter API since at least v3)
- Fix README.md multi-arch claim (images are ARM64-only, not amd64/arm64)
- Document leaderlog.nonceIntegrityCheck and leaderlog.backfillSchedules in
  config.yaml.example (both existed in code but were undiscoverable from config)
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 8, 2026

Warning

Rate limit exceeded

@wcatz has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 2 minutes and 10 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 221c2b66-5442-452e-accc-79cef5783b4f

📥 Commits

Reviewing files that changed from the base of the PR and between bdefc3a and 81cd57b.

📒 Files selected for processing (5)
  • cli.go
  • commands.go
  • docs/twitter-setup.md
  • main.go
  • nonce.go
📝 Walkthrough

Walkthrough

Consolidates stake-fetching for leaderlog/nextblock into a new Indexer method with NtC→Koios fallback, removes nonce-streaming and several nonce-related Store/DB APIs and tests, simplifies DB integrity result fields, refactors duck media fetching, updates README and Twitter docs, and adds commented leaderlog config options.

Changes

Cohort / File(s) Summary
Stake retrieval
commands.go, main.go
Added Indexer.queryStakeForLeaderlog and replaced scattered NtC/Koios stake-fetch logic in cmdLeaderlog and cmdNextBlock with calls to it; adjusted leaderlog-related stake fetching paths.
Nonce streaming & store API removals
store.go, db.go, store_test.go, comprehensive_test.go
Removed BlockNonceRows interface, StreamBlockNonces, GetNonceValuesForEpoch, GetLastBlockHashForEpoch, related sqlite iterator types/implementations, and associated tests.
Nonce helpers & types
nonce.go, integrity.go
Removed IntegrityResult type and fetchLastBlockHashFromKoios; simplified GetNonceForEpoch fallback logging; removed Valid and Repaired fields from DBIntegrityResult.
Local query API
localquery.go
Removed NodeQueryClient.QueryStakeDistribution method and its usage; stake snapshot retrieval via GetStakeSnapshots remains.
Duck media & Koios URL cleanup
main.go
Added fetchDuckURL helper and Indexer.fetchDuckByType; consolidated duck media fetching and simplified Koios URL constructions across several helpers.
Docs & config
README.md, docs/twitter-setup.md, config.yaml.example
Updated README text about ARM64 image publishing; replaced Twitter/X setup guide with a concise opt-in config block; added commented nonceIntegrityCheck and backfillSchedules leaderlog options in example config.

Sequence Diagram(s)

sequenceDiagram
  participant CLI as CLI (cmdLeaderlog / cmdNextBlock)
  participant Indexer as Indexer.queryStakeForLeaderlog
  participant NtC as NtC Snapshot Service
  participant Koios as Koios API

  CLI->>Indexer: request stake for targetEpoch
  Indexer->>NtC: fetch snapshots (mark/set/go) for epoch
  alt NtC returns stake
    NtC-->>Indexer: poolStake, totalStake
    Indexer-->>CLI: return stakes
  else NtC missing
    Indexer->>Koios: fetch pool stake & total stake for epoch
    alt Koios returns stake
      Koios-->>Indexer: poolStake, totalStake
      Indexer-->>CLI: return stakes
    else Koios fails
      Koios-->>Indexer: error
      Indexer-->>CLI: error (no stake data)
    end
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

🐇 I hopped through code with nimble feet,

Nonces trimmed back from every street.
Stakes now asked from one neat place,
Ducks fetched clean with tidy grace.
A quiet repo — simpler beat. 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: removing dead code, fixing a stake bug, and consolidating helper functions, which matches the primary objectives of the PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch audit/cleanup-dead-code-and-docs

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@commands.go`:
- Around line 392-456: calculateAndPostLeaderlog still implements the old
fallback epoch loop ([epoch-1, epoch-2, epoch-3]) instead of using the
centralized stake lookup, causing wrong-epoch stake results; update
Indexer.calculateAndPostLeaderlog in main.go to call
i.queryStakeForLeaderlog(ctx, targetEpoch) (the same helper used by /leaderlog
and /nextblock), handle its returned (poolStake, totalStake, err) and use that
result or error path, and remove the local fallback loop and its Koios/NtC logic
so all leaderlog code relies on queryStakeForLeaderlog.

In `@docs/twitter-setup.md`:
- Around line 33-40: The fenced code block listing the environment variable
names lacks a language tag (causing MD040); update the fence for that block (the
triple-backtick around the TWITTER_API_KEY, TWITTER_API_KEY_SECRET,
TWITTER_ACCESS_TOKEN, TWITTER_ACCESS_TOKEN_SECRET lines) to include an explicit
language tag such as text (i.e., change ``` to ```text) so markdownlint stops
flagging MD040.
- Around line 44-53: The docs incorrectly document the duck.media value as "img"
but Indexer.getDuckMedia() expects "image"; update the documentation text
describing duck.media (the list items for media options) to replace the "img"
entry with "image" so setting duck.media="image" will select JPEG images as
intended and match the Indexer.getDuckMedia() behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 620f4ee8-146a-4e86-899e-5f4f319faf6e

📥 Commits

Reviewing files that changed from the base of the PR and between 172dcb6 and f5b0e46.

📒 Files selected for processing (12)
  • README.md
  • commands.go
  • comprehensive_test.go
  • config.yaml.example
  • db.go
  • docs/twitter-setup.md
  • integrity.go
  • localquery.go
  • main.go
  • nonce.go
  • store.go
  • store_test.go
💤 Files with no reviewable changes (5)
  • localquery.go
  • comprehensive_test.go
  • store_test.go
  • db.go
  • store.go

- calculateAndPostLeaderlog() now uses queryStakeForLeaderlog() helper
  instead of its own inline NtC+Koios block; this was the last divergent
  stake-fetch path and the actual auto-trigger code path that fires at 60%
  epoch progress (commands.go btnLeaderlogNext is a separate on-demand path)

- docs/twitter-setup.md: correct duck.media value 'img' -> 'image' throughout
  ('img' falls through to the 'both' branch; 'image' is what getDuckMedia() checks)

- docs/twitter-setup.md: add 'text' language tag to bare fenced block (MD040)
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/twitter-setup.md`:
- Around line 55-62: The docs currently state that omitting the `twitter` block
disables Twitter, but in reality `main.go` treats an omitted `twitter.enabled`
as an env-driven opt-in (it checks for `TWITTER_*` env vars), so only
`twitter.enabled: false` is a hard-disable; update the "Disabling" section to
explicitly state that (1) setting `twitter: enabled: false` is the only
guaranteed way to disable posting, and (2) omitting the block will still enable
Twitter if any `TWITTER_*` environment variables are present (reference the
behavior in `main.go` where omitted config is overridden by env vars).

In `@main.go`:
- Around line 1856-1857: The helpers build Koios URLs from the global
koiosRestBase, causing mainnet queries even when Start() set i.koios for a
different network; update the call sites (e.g. where backfillSchedules() and
buildLeaderlogHistory() call fetchTotalStakeFromKoios,
fetchPoolRegistrationEpoch, fetchPoolStakeFromKoios, fetchPoolForgedSlots) to
pass the network-selected base URL (i.koios) and change each helper signature to
accept that baseURL parameter and use it instead of the global koiosRestBase so
all Koios calls target the correct network.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8e9bac9d-b214-440f-8879-d2a84b0cbd2a

📥 Commits

Reviewing files that changed from the base of the PR and between f5b0e46 and bdefc3a.

📒 Files selected for processing (2)
  • docs/twitter-setup.md
  • main.go

wcatz added 2 commits March 8, 2026 08:04
Helper belongs with calculateAndPostLeaderlog and other core leaderlog
logic in main.go, not buried in commands.go among Telegram handlers.

NtC timeout reduced from 5m to 30s. NtC is known to time out in some
deployments (K8s NtC socket issues); a 5-minute stall before Koios
fallback was delaying every automated leaderlog post. 30s is enough for
a healthy NtC connection and fails fast when it's not available.
koiosRESTBase network routing:
- Replace global koiosRestBase constant (mainnet-only tosidrop URL) with a
  koiosRESTBase(networkMagic int) helper function that returns the correct
  Koios REST endpoint per network:
    preprod  -> https://preprod.koios.rest/api/v1
    preview  -> https://preview.koios.rest/api/v1
    mainnet  -> https://koios.tosidrop.me/api/v1  (unchanged)
- fetchPoolRegistrationEpoch, fetchPoolStakeFromKoios, fetchTotalStakeFromKoios,
  fetchPoolForgedSlots all take baseURL string param; callers in
  buildLeaderlogHistory (main.go) and runHistoryCommand (cli.go) pass
  koiosRESTBase(networkMagic)
- NonceTracker.fetchNonceFromKoios uses koiosRESTBase(nt.networkMagic)
- Previously all five REST helpers hardcoded mainnet, breaking history/backfill
  on preprod and preview

docs/twitter-setup.md:
- Correct hard-disable section: omitting twitter block is NOT a hard disable;
  enabled: false is the only guaranteed way to prevent Twitter posts (backward
  compat mode enables Twitter when env vars are present and block is absent)
@wcatz wcatz merged commit e5a8da2 into master Mar 8, 2026
5 checks passed
@wcatz wcatz deleted the audit/cleanup-dead-code-and-docs branch March 20, 2026 14:23
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.

1 participant