Fall back to reading Pyth's price objects when Hermes is unreachable

[JustaLiang]

Problem

aggregateBasicPrices rejected the moment the Hermes fetch failed, so an outage at Pyth's off-chain endpoint took down every SDK path that prices collateral — getOraclePrices, PSM swaps, and any borrow or withdraw.

Nothing about that rejection was load-bearing. pyth_rule::feed reads the PriceInfoObject with get_price_unsafe and hands the aggregator option::none() when the reading is past that coin type's tolerance, so a stale price is never collected; aggregate then aborts ERiskyPrice unless the surviving sources clear the weight threshold. The freshness decision already lives on-chain. Refusing to build the PTB only denied the chain the chance to make it.

Change

On a Hermes failure the PTB is now built against the price objects as they already stand.

The rule is still fed — remove_outliers aborts EMissingPriceSource unless every weighted rule was collected in that PTB, and abstaining counts as collected while omitting the call does not. Only the VAA update that would have preceded it is dropped, along with its wormhole verification and per-feed fee.

• pythStaleReadFallback: false keeps the old behaviour.
• Every engagement is reported through onPythStaleRead, defaulting to a console.error so the degradation is never silent.
• Resolving the price object id is now its own function (resolvePythPriceInfoObjectIds) rather than a by-product of building the update calls — it is a pure on-chain lookup and answers fine when Hermes does not. Both paths share the PythCache, so the fallback usually costs no extra RPC.

Verified against mainnet

Dry-run with PRICE_SERVICE_ENDPOINT pointed at a closed port:

	Hermes reachable	Hermes down
SUI	0.64750581	0.64750603
USDC	0.99989	0.99987919
BTC	64178.35791083	64178.33

All three price successfully with Hermes fully unreachable, within 0.0001% of baseline — the shared price objects are refreshed constantly by other protocols' transactions, well inside the 30s tolerance.

Scope — what this covers today

• ✅ Hermes outage while the price object is still fresh. The common case on busy feeds, demonstrated above.
• ⏳ Outage lasting past the tolerance, where the reading really is stale and a second source has to carry the aggregate. This needs SupraRule weight, which is currently going through the multisig (pnpm supra:weights). supra:read shows every aggregator at threshold 1 with exactly one weighted rule today, so until that lands, a genuinely stale reading degrades to an on-chain ERiskyPrice rather than an SDK throw. Same failure, better diagnostics — and the SDK side is then already in place.

No Move changes needed; pyth_rule and aggregator already behave correctly.

Bug fixed along the way

buildPythPriceUpdateCalls threw Price feed … not found after parse_and_verify, the accumulator call and the fee split were already appended — stranding a hot potato in a PTB that could never build, and impossible to recover from by catching. All validation is hoisted above the first mutation, pinned by a test asserting the transaction is untouched when it throws. This is also what makes the fallback safe to wrap in a try/catch.

Test plan

• pnpm test:unit — 126 pass (12 new: fallback engages/reports/respects the opt-out, does not swallow update-path errors, resolver behaviour, no-poison guarantee)
• pnpm vitest run test/e2e/oracle.test.ts — 5 pass against mainnet
• pnpm lint, pnpm build
• Live dry-run with the endpoint forced to fail (table above)

🤖 Generated with Claude Code

[waterlabs-bot]

Code review

Found 2 non-blocking issues:

1. Async stale-read handlers can become unhandled rejections. onPythStaleRead is typed as returning void, but TypeScript accepts an async function at that call site. The returned promise is discarded, so a rejected telemetry/reporting callback becomes an unhandled rejection (and can terminate Node under strict/default process policy) while a synchronous throw propagates through aggregateBasicPrices. Please make the callback contract consistent: either accept void | Promise<void> and await it, or isolate/log both synchronous throws and promise rejections if this hook is intended to be observational only.

bucket-protocol-sdk/src/client.ts

Lines 1170 to 1176 in 572435f

	try {
	updateData = await fetchPriceFeedsUpdateDataFromHermes(config.PRICE_SERVICE_ENDPOINT, pythPriceIds);
	} catch (cause) {
	if (!this.pythStaleReadFallback) throw cause;
	this.onPythStaleRead({ feedIds: pythPriceIds, cause });
	return resolvePythPriceInfoObjectIds(this.suiClient, config.PYTH_STATE_ID, pythPriceIds, this.pythCache);
	}

2. The new fallback path is not exercised end-to-end by automated tests. These tests mock both the Hermes failure and resolvePythPriceInfoObjectIds, while the normal update builder is mocked suite-wide. The existing oracle E2E only covers the reachable-Hermes path, so CI does not preserve the key sequence: Hermes fails → the real on-chain resolver supplies the PriceInfoObject → pyth_rule::feed receives it → the fallback PTB dry-run succeeds. Please add a focused E2E/integration case with a deliberately unreachable endpoint and assert the callback plus a successful price/dry-run. (I ran that exact mainnet SUI probe against this SHA and it succeeded; this is a regression-coverage gap, not evidence that the implementation currently fails.)

bucket-protocol-sdk/test/unit/price-aggregation.test.ts

Lines 263 to 272 in 572435f

	const hermesDown = () =>
	vi
	.spyOn(pyth, 'fetchPriceFeedsUpdateDataFromHermes')
	.mockRejectedValue(new Error('Hermes price fetch failed: 503 upstream'));
	/** Resolve ids the same way the mocked update path does, so the two are comparable. */
	const mockResolve = () =>
	vi
	.spyOn(pyth, 'resolvePythPriceInfoObjectIds')
	.mockImplementation(async (_client, _state, feedIds) => [...feedIds]);

Severity/confidence: both Medium · High confidence. No blocking protocol-safety issue found: the deployed pyth_rule::feed freshness check collects none for stale readings, and aggregator::remove_outliers counts the rule as present before dropping abstentions and enforcing the surviving weight threshold.

🤖 Generated with Hermes Agent

[JustaLiang]

Both findings addressed in 6c823f8 — thanks, they were fair.

1. Async stale-read handlers → unhandled rejections. Fixed, and the sharper half of this was the synchronous case: a throw escaping the hook propagated straight out of aggregateBasicPrices, so a broken metrics call turned a survivable Hermes outage into exactly the hard failure the fallback exists to prevent. Both paths are now contained in reportPythStaleRead, and the type says void | Promise<void> so the async case is honest rather than merely tolerated.

Deliberately not awaited — reporting is observational, and PTB construction shouldn't block on someone's telemetry endpoint. Failures from the hook go to console.error noting the fallback is unaffected.

Two unit tests cover it, and I verified they're not vacuous by reverting the wrapper: both fail without it, the async one surfacing as a process-level unhandled rejection.

2. Fallback not exercised end-to-end. Agreed — and thanks for running the mainnet probe to confirm it was a coverage gap rather than a live failure. New E2E case in test/e2e/oracle.test.ts points PRICE_SERVICE_ENDPOINT at a closed port and asserts the full sequence: callback fires → real on-chain resolver supplies the PriceInfoObject → pyth_rule::feed receives it → dry-run succeeds → price > 0. It also asserts the absence of every command the VAA update would have added (vaa::parse_and_verify, update_single_price_feed, the accumulator call), so it can't pass by Hermes quietly having worked.

One caveat worth stating: that test leans on mainnet's SUI price object sitting inside pyth_rule's 30s tolerance, which other protocols' transactions keep it well within. If it ever does go stale the test aborts ERiskyPrice — which is the correct outcome to observe, since SupraRule has no aggregator weight yet and no second source can carry the price. Noted in the test docblock so a future failure reads as signal rather than flake.

Verification: pnpm lint, pnpm build, pnpm test:unit (128 pass, +2), pnpm vitest run test/e2e/oracle.test.ts (6 pass, +1) against mainnet.

Unrelated: the enable-auto-merge check is failing on 401 Bad credentials for the workflow's GH_TOKEN — a repo secret issue on the default branch, not from this PR. lint, e2e-test and check-secrets are green.

[waterlabs-bot]

Follow-up review

Both prior findings are resolved at 6c823f8f9e241c310b28fceafc14be50c7cf15e2.

• F-01 resolved: reportPythStaleRead contains synchronous throws and attaches a rejection handler immediately for async callbacks, while keeping telemetry fire-and-forget. Focused unit coverage proves both failure modes leave the fallback PTB intact and do not emit an unhandled rejection.
• F-02 resolved: the new mainnet E2E forces Hermes offline, uses the real on-chain resolver, requires pyth_rule::feed, excludes all Pyth/Wormhole update calls, dry-runs the PTB successfully, and verifies a positive price.

No new blocking or publishable non-blocking issues found in the follow-up delta. CI at this head passed 190 tests (2 skipped), including all 6 oracle E2E cases; lint and secret scan are green. I also reran the focused unit test, the fallback E2E repeatedly, TypeScript checking, and the full package build successfully. The failing enable-auto-merge check remains an unrelated workflow PAT 401.

🤖 Generated with Hermes Agent

[waterlabs-bot]

Follow-up correction

One additional Medium · High-confidence public-type issue surfaced in the completed independent check:

• onPythStaleRead was changed from (event) => void to (event) => void | Promise<void>. In TypeScript, the original void callback intentionally accepts value-returning handlers, while the union does not. Common callbacks such as event => events.push(event) or logger/counter functions now fail with TS2322 even though their return value is ignored. This was reproduced with TypeScript 5.9.2.

Please retain (event: PythStaleReadEvent) => void in the public signatures/private field (the runtime Promise.resolve(...) wrapper still observes actual async returns and catches rejections), or use an unknown return type if accepting arbitrary observational callback results should be explicit.

bucket-protocol-sdk/src/client.ts

Lines 92 to 95 in 6c823f8

	private network: Network;
	private pythCache = new PythCache();
	private pythStaleReadFallback: boolean;
	private onPythStaleRead: (event: PythStaleReadEvent) => void | Promise<void>;

bucket-protocol-sdk/src/client.ts

Lines 125 to 128 in 6c823f8

	config?: ConfigType;
	configOverrides?: Partial<ConfigType>;
	pythStaleReadFallback?: boolean;
	onPythStaleRead?: (event: PythStaleReadEvent) => void | Promise<void>;

bucket-protocol-sdk/src/client.ts

Lines 176 to 179 in 6c823f8

	config?: ConfigType;
	configOverrides?: Partial<ConfigType>;
	pythStaleReadFallback?: boolean;
	onPythStaleRead?: (event: PythStaleReadEvent) => void | Promise<void>;

This is non-blocking at runtime, but it should be corrected before publishing the SDK API. My earlier approval remains recorded; this comment corrects the statement that the follow-up delta had no publishable issues.

🤖 Generated with Hermes Agent