Skip to content

feat(acp-adapter): expose SPHERE_AGGREGATOR_URL + SPHERE_TRUSTBASE_URL env overrides - #3

Open
vrogojin wants to merge 2 commits into
mainfrom
feat/aggregator-trustbase-env-override
Open

feat(acp-adapter): expose SPHERE_AGGREGATOR_URL + SPHERE_TRUSTBASE_URL env overrides#3
vrogojin wants to merge 2 commits into
mainfrom
feat/aggregator-trustbase-env-override

Conversation

@vrogojin

Copy link
Copy Markdown
Contributor

Summary

The faucet's network preset hardcodes the testnet aggregator and trust base. When running against a self-hosted aggregator (e.g. sphere-sdk's tests/e2e/local-infra deployment, or a private wallet's local stack), the operator needs to redirect both without forking the SDK to add a custom network preset.

This patch wires through the SDK's existing oracle.url + oracle.skipVerification fields and adds three pass-through env vars:

env var effect
SPHERE_AGGREGATOR_URL overrides the network preset's aggregator URL
SPHERE_TRUSTBASE_URL overrides the hardcoded testnet trust-base GitHub URL
SPHERE_AGGREGATOR_SKIP_VERIFICATION bypass client-side trust-base verification (defaults ON when AGGREGATOR_URL is set; override with =false)

All three are optional. Without any new vars set, faucet behavior is byte-for-byte identical to today.

Logs a new aggregator_override_active line when the URL is set so operators can confirm the redirect took effect.

Companion

sphere-sdk PR #324 adds matching --aggregator-url, --trustbase-url, --nametag, --skip-verification flags to run-faucet.sh (the SSL-wrapped wrapper) which forward these env vars into the container.

Test plan

  • Without env vars set: faucet boots identically (verified — uses testnet aggregator)
  • With SPHERE_AGGREGATOR_URL=https://aggregator-unicity-dev.dyndns.org: aggregator_override_active logged, faucet's oracle hits our self-hosted aggregator instead of the testnet one (verified)
  • With SPHERE_NAMETAG=xaleava: NAMETAG_BINDING (kind 30078) event verified on relay
  • Type-check: tsc --noEmit passes (verified locally)

Known limitation

Even with the overrides, Sphere.init's nametag mint silently no-ops when skipVerification is on — the relay event publishes but no L3 commitment lands on the aggregator, so identity.nametag returns null. This is an SDK-level concern (Sphere.init swallowing the mint step) and isn't introduced by this PR.

…L env overrides

The faucet's network preset hardcodes the testnet aggregator and the
testnet trust base. When running against a self-hosted aggregator
(e.g. sphere-sdk's tests/e2e/local-infra deployment), the operator
needs to redirect both without forking the SDK to add a custom
network preset.

This patch wires through the SDK's existing oracle.url +
oracle.skipVerification fields and adds the corresponding env vars:

  SPHERE_AGGREGATOR_URL          (was hardcoded to network preset)
  SPHERE_TRUSTBASE_URL           (was hardcoded GitHub URL)
  SPHERE_AGGREGATOR_SKIP_VERIFICATION  (defaults on when AGGREGATOR_URL set)

Existing env vars (SPHERE_NOSTR_RELAYS, SPHERE_NAMETAG, etc.) are
unchanged. Without any new vars set, the faucet behaves exactly as
before.

Logs gain `aggregator_override_active` line when the URL is set so
the operator can confirm the redirect actually took effect.

Companion to sphere-sdk PR #324 (run-faucet.sh CLI flags).

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request introduces configuration overrides via environment variables in src/acp-adapter/main.ts. Specifically, it allows overriding the trustbase URL using SPHERE_TRUSTBASE_URL, the aggregator URL using SPHERE_AGGREGATOR_URL, and the verification skip behavior using SPHERE_AGGREGATOR_SKIP_VERIFICATION. The review feedback recommends robustly parsing these environment variables by trimming whitespace and handling empty strings to prevent runtime errors and ensure correct fallback behavior.

Comment thread src/acp-adapter/main.ts
// ---------------------------------------------------------------------------
log.info({ url: TRUSTBASE_URL }, 'downloading_trustbase');
const tbResponse = await fetch(TRUSTBASE_URL, { signal: AbortSignal.timeout(30_000) });
const trustbaseUrl = process.env['SPHERE_TRUSTBASE_URL'] ?? DEFAULT_TRUSTBASE_URL;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If the SPHERE_TRUSTBASE_URL environment variable is set to an empty string (e.g., in a Docker or Kubernetes configuration), the nullish coalescing operator (??) will not fall back to DEFAULT_TRUSTBASE_URL. This results in trustbaseUrl being "", causing fetch to throw a runtime error on startup. Trimming the value and using a logical OR (||) fallback prevents this issue.

Suggested change
const trustbaseUrl = process.env['SPHERE_TRUSTBASE_URL'] ?? DEFAULT_TRUSTBASE_URL;
const trustbaseUrl = process.env['SPHERE_TRUSTBASE_URL']?.trim() || DEFAULT_TRUSTBASE_URL;

Comment thread src/acp-adapter/main.ts
Comment on lines +149 to +157
const aggregatorUrl = process.env['SPHERE_AGGREGATOR_URL'];
if (aggregatorUrl) {
log.info({ url: aggregatorUrl }, 'aggregator_override_active');
}
const skipVerification = (() => {
const v = process.env['SPHERE_AGGREGATOR_SKIP_VERIFICATION'];
if (v === undefined) return aggregatorUrl ? true : undefined;
return v === '1' || v.toLowerCase() === 'true';
})();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Environment variables can sometimes contain accidental leading/trailing whitespace or be defined as empty strings in containerized environments. Trimming SPHERE_AGGREGATOR_URL and SPHERE_AGGREGATOR_SKIP_VERIFICATION ensures robust parsing. Additionally, treating an empty string for SPHERE_AGGREGATOR_SKIP_VERIFICATION as undefined allows it to correctly fall back to the default behavior.

Suggested change
const aggregatorUrl = process.env['SPHERE_AGGREGATOR_URL'];
if (aggregatorUrl) {
log.info({ url: aggregatorUrl }, 'aggregator_override_active');
}
const skipVerification = (() => {
const v = process.env['SPHERE_AGGREGATOR_SKIP_VERIFICATION'];
if (v === undefined) return aggregatorUrl ? true : undefined;
return v === '1' || v.toLowerCase() === 'true';
})();
const aggregatorUrl = process.env['SPHERE_AGGREGATOR_URL']?.trim() || undefined;
if (aggregatorUrl) {
log.info({ url: aggregatorUrl }, 'aggregator_override_active');
}
const skipVerification = (() => {
const v = process.env['SPHERE_AGGREGATOR_SKIP_VERIFICATION']?.trim();
if (v === undefined || v === '') return aggregatorUrl ? true : undefined;
return v === '1' || v.toLowerCase() === 'true';
})();

… URL is set

The original patch defaulted skipVerification=true when
SPHERE_AGGREGATOR_URL was set, on the theory that a self-hosted
aggregator's trust base wouldn't match the SDK's defaults. That's
WRONG: skipVerification bypasses the SDK's trust-base loader
entirely, so `oracle.getTrustBase()` returns null. Every
operation that needs the trust base then fails with:

  "Trust base not available. Oracle provider must implement getTrustBase()"

This breaks nametag mint, invoice mint, and inclusion-proof
verification — exactly the operations a self-hosted aggregator
deployment wants to use.

The right behavior when redirecting the aggregator is to ALSO
supply SPHERE_TRUSTBASE_URL pointing at the aggregator's trust
base. sphere-sdk PR #326 makes the aggregator-proxy serve it
under /.well-known/trust-base.json so the URL is always
available.

skipVerification stays opt-in via SPHERE_AGGREGATOR_SKIP_VERIFICATION.

Verified: with this change + SPHERE_TRUSTBASE_URL set,
xaleava nametag mints successfully against the local-infra
aggregator (1 commitment in mongo, NAMETAG_BINDING event on
relay, identity.nametag="xaleava" in discovery doc).
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