Skip to content

Add scanner commands - #111

Open
martinssvlabs wants to merge 22 commits into
ssvlabs:mainfrom
martinssvlabs:scanner-integration
Open

Add scanner commands#111
martinssvlabs wants to merge 22 commits into
ssvlabs:mainfrom
martinssvlabs:scanner-integration

Conversation

@martinssvlabs

@martinssvlabs martinssvlabs commented Apr 1, 2026

Copy link
Copy Markdown

Description

This PR integrates scanner capabilities into ssv-keys while preserving the existing shares workflow.

What changed

  • Added scanner commands: nonce, cluster, operator.
  • Updated packaging scripts to always run a fresh build before pkg on Linux/macOS/Windows.
  • Updated README binary usage examples.

IMPORTANT: this PR temporarily keeps "@ssv-labs/ssv-sdk": "github:ssvlabs/ssv-sdk" until the next SDK version is published to npm. Once released, we will switch to the corresponding npm semver version.

Comment thread src/commands/actions/ClusterAction.ts Outdated
Comment thread src/scanner/ClusterScanner.ts Outdated
Comment thread src/scanner/OperatorScanner.ts
@martinssvlabs

Copy link
Copy Markdown
Author

CI failures are expected on this PR and are related to the SDK dependency version, not the ssv-keys changes themselves.

This branch currently points @ssv-labs/ssv-sdk to the SDK GitHub ref/branch, and GitHub Actions is failing during pnpm install while trying to prepare that git-hosted package. Once the SDK changes are merged and ssv-keys is updated to consume the proper published SDK version in package.json, these CI errors should go away.

@Chris-ssvlabs

Copy link
Copy Markdown

Issues

  1. Address validation is redundant and incomplete (BaseScanner.ts:30-35)

Manual length/prefix checks fire before getAddress(). getAddress() already
validates everything and throws a clear viem error. The manual checks add noise
but miss non-hex chars (e.g. 0x + 40 garbage chars passes length check, then
throws an opaque viem error).

// Drop lines 30-35; just let getAddress() throw
this.params.ownerAddress = getAddress(scannerParams.ownerAddress);

  1. ClusterAction JSON output bug (ClusterAction.ts:50)

cluster: Object.values(result.cluster), // ← array of values, no keys
Outputs [1, "0", "0", true, "0"] — useless without field names. Should be
result.cluster directly.

  1. OperatorScanner.getOwnerOperators is public (OperatorScanner.ts:29)

It takes a raw SSVSDK instance, which callers don't have. Should be private.
Accidentally leaks an internal method as part of the public API.

  1. Triple build in package-all (package.json:46-48)

"package-linux": "pnpm run build && pkg ...",
"package-macos": "pnpm run build && pkg ...",
"package-win": "pnpm run build && pkg ...",
package-all runs 3 sequential pnpm run builds. Should be one build at the top:
"package-all": "pnpm run build && pnpm run package-linux && pnpm run
package-macos && pnpm run package-win"
and strip pnpm run build && from each individual script.

  1. DEFAULT_CLUSTER_SNAPSHOT strings vs bigint (ClusterScanner.ts:38-44)

const DEFAULT_CLUSTER_SNAPSHOT: ClusterSnapshotData = {
validatorCount: "0", // ← string
networkFeeIndex: "0", // ← string
...
};
ClusterSnapshotData is Pick<SdkClusterSnapshot, ...> — if the SDK types these as
bigint, this silently lies to the type system. Verify the SDK's return types
const DEFAULT_CLUSTER_SNAPSHOT: ClusterSnapshotData = {
validatorCount: "0", // ← string
networkFeeIndex: "0", // ← string
...
};
ClusterSnapshotData is Pick<SdkClusterSnapshot, ...> — if the SDK types these as bigint, this silently
lies to the type system. Verify the SDK's return types and use 0n if needed.

  1. Vague validation error (shared/operator-ids.ts:21-23)

"Comma-separated list of operator IDs. The amount must be 3f+1 compatible."
Users don't know what "3f+1 compatible" means. Should say: "Operator count must be 4, 7, 10, or 13."

  1. Unknown flags silently enter interactive mode (resolve-binary-mode.ts:52-59)

ssv-keys --some-typo falls through to { interactive: true } instead of showing an error/help. Probably
intentional but could confuse users who expect flag validation.

  1. No user-friendly error wrapping in actions

Scanner calls (clusterScanner.run(...), etc.) have no try/catch. Raw viem/SDK errors hit the user on
bad node URLs, wrong networks, or network failures. At minimum wrap and print a clean message.


Minor

  • operator.ts:22 — console.log("error:", error) debug log left in production path
  • OperatorScanner:31 — passes ownerAddress.toLowerCase() to SDK but stores checksummed; inconsistency
    is fine but worth a comment
  • dist/ committed to repo — existing pattern, but makes this PR's diff unreadable

Tests

Coverage is reasonable for the new units. Missing:

  • Error paths (bad node URL, SDK failure, invalid address passed to scanner)
  • OperatorAction when SDK returns 0 clusters
  • ClusterAction output format assertions (would have caught the Object.values bug)

Summary

Solid architecture, clean separation, good backwards compat. Fix the Object.values JSON bug (#2) and
triple-build (#4) before merge — both are real bugs. The rest are quality improvements worth a
follow-up.

@Chris-ssvlabs

Copy link
Copy Markdown

One last thing - getScannerErrorMessage is duplicated across 3 files - unify it.

@Chris-ssvlabs Chris-ssvlabs left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

getScannerErrorMessage

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.

5 participants