Add CLI on top of config keys refactor - #6082
paullinator wants to merge 17 commits into
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
67406a5 to
35eeb43
Compare
a8819b9 to
60adf1e
Compare
7d3a2ba to
c0c5f13
Compare
0574066 to
4bb295e
Compare
4bb295e to
70e9045
Compare
1f45b00 to
c7a71d4
Compare
e3c597d to
784c6ed
Compare
`typechain` emits `export * as factories from './factories'`, and the React Native preset does not transform namespace re-exports. The plugin was declared but never enabled, so `TransactionListTop` failed to parse the moment `src/plugins/contracts` existed — which it does after any `npm install`, since `prepare` generates it. Metro needs that transform as much as jest does, so it belongs in the shared config.
src/util/hmacAuth.ts imports hashjs directly, but the package was only ever resolved transitively. Declare it so a clean install and the Node CLI bundle both get it.
`network.ts`, `utils.ts` and the locale boot each pulled React Native in through their module load paths, so nothing outside the app could fetch from the info server, format an amount, or pick a language table. Fiat constants and helpers move to `fiatConstants.ts`, and `getOsVersion` to `rnUtils.ts` alongside the other React Native-only helpers, with `keysStore.ts` following it there. `network.ts` takes its server lists and device fields through `configureNetwork` and `initInfoServer(params)` rather than reading `appConfig` and `react-native-device-info` at module scope. Locale boot splits the same way: `bootLocale.ts` applies a language table and number format with no React Native imports, and `initLocale.ts` stays GUI-only, feeding it what `react-native-localize` reports. Capturing those device fields is separate from starting the poll: `configureInfoServer` records them synchronously, so `fetchPublicRollup` works for whoever calls it first, while `initInfoServer` owns the polling and the decision to skip the unsigned launch fetch. `fetchWaterfall` refuses an empty server list rather than handing it to `asyncWaterfall`, which awaits `Promise.race([])` and never settles.
Exchange rates could not be fetched outside the app: the module read its server list and its error reporter at load time, both of which come from React Native. The query logic now takes what it needs through `configureExchangeRates`, and `exchangeRatesGui` supplies the GUI's Airship reporter at app start. A Node caller supplies its own, so the same rate lookup answers the same way in both places.
`initLocale` reaches for `react-native-localize`, so nothing outside the app could ask which locale to use. The decision itself is pure: read a tag from argv, config or the environment, normalize it, and pick a language table. `nodeLocale.ts` holds that decision with no React Native imports, and feeds the same `applyLocale` that the GUI's device lookup already calls, so the GUI and any Node caller resolve a locale the same way rather than approximately the same way. Precedence is explicit and tested: an explicit tag, then config, then `EDGE_CLI_LOCALE`, then `LC_ALL` / `LC_MESSAGES` / `LANG`, then `Intl`, then `en-US`. `es_MX.UTF-8@euro` and `C` both resolve, which is what the POSIX forms actually look like. `env` is typed as the variables it reads rather than `NodeJS.ProcessEnv`, which in this repo demands `NODE_ENV` and would make every caller invent one.
`CategoriesActions.ts` held five hundred lines deciding what a transaction should be called: the category, the payee, the direction, and the label for each action type. All of it is a pure function of the transaction, the wallet and the account, but it sat behind Redux imports, so nothing outside the app could ask the same question and get the same answer. `src/util/txDisplay/` holds that logic now — `displayInfo` for the derivation, `category` for the category strings, `txActionLabels` for the action names, and `currencyCodes` for the ticker lookups. `CategoriesActions` re-exports what the GUI already imported, so no scene changed. The point is that two callers cannot drift. A transaction rendered in a list, exported to CSV, or printed by a script now describes itself identically, because it is the same code deciding.
Three small pieces of GUI state that any caller reading transactions needs, and none of which had a reason to be Redux-only. `exchangeDenom` picks the denomination a currency or token reports amounts in. `DenominationSelectors` keeps its selector shape and calls it, so the two cannot disagree about what a multiplier is. `spamThreshold` decides which incoming transactions are dust worth hiding. The GUI applies it to every list; a caller that reads the same wallet and does not apply it sees a different set of transactions, which is the sort of difference that looks like a bug in whichever one you did not write. `localAccountSettings` reads the device-local settings file that holds the spam filter toggle, and `LocalSettingsActions` reads through it rather than duplicating the format. The threshold needs a rate, and asks for it at the current hour rather than the current millisecond, so repeated listings share one cache entry instead of missing on every call. It is a different source from the GUI's, which reads live rates from Redux, and a lookup that fails yields no filtering — stated in the module, because the two can legitimately disagree. `syncedSettingsFile` holds the one definition of the synced `Settings.json` that Node-safe code reads. The GUI's own cleaner sits behind an Airship import and cannot be loaded here, so this is a two-field view of the same file with the same defaults, and a test asserts those defaults still match the GUI's.
`TransactionExportActions.tsx` was a five-hundred-line thunk that did four separable jobs: fill in historical fiat values, render CSV, render QBO, and render the Bitwave format. Only the last step needed React Native, and only for writing the file. `fillTxsFiat` asks the rates server what each transaction was worth on the day it happened, which is the part that makes an export more than a dump of native amounts. `txExport/format` renders the three formats. `exportTxInfo` holds the Bitwave account mapping the exporter needs. The thunk keeps the file-writing and the share sheet, and calls the same renderers. `TransactionsExportScene` follows it. A test now covers `fillTxsFiat` against a partial wallet, which is all it reads. The formats matter here: an export that a person reconciles against their books has to be byte-identical whichever tool produced it, and the only way to be sure of that is for one renderer to produce both.
`SendScene2` saved a sent transaction and then attached its metadata, its category, its notes and any swap details in a sequence that had to happen in a particular order and had grown inline in the scene. A second caller that saved a transaction and got the order wrong would produce a transaction that looks right until someone exports it. `txTagging/apply` holds that sequence. `SendScene2` calls it and loses two dozen lines. The scene was the only definition of what a correctly tagged transaction is, and now it is not the only caller that can produce one. It re-applies only the fields the caller actually supplied. Under `EdgeMetadataChange` an empty string is a value rather than "leave unchanged", so passing all three through would let a caller who set one field erase the other two that core derived. The trigger is any non-empty name, notes or category, which is wider than the scene's old `payeeName != null` check and preserves a category that would otherwise be lost; callers pass the metadata they were given, never computed display metadata.
A long-lived engine daemon owns the `EdgeContext` and answers a JSON
REST API over a Unix socket; the `edge-cli` binary is a thin one-shot
client that spawns the engine on demand and keeps a session id in
`session.json` so commands chain. `docs/EDGE_CLI.md` describes that
architecture and deliberately documents no endpoints — the reference is
generated.
The point of this commit is the declaration format, so it carries
thirteen calls. Each is one `route({…})`: the core call it fronts, the
HTTP method and path, how it appears on the command line, cleaners for
the query, body and response, and its error codes. The prose lives
inside the declaration, beside the field it describes, and the JSDoc
above carries what belongs to the call as a whole.
Nothing is written twice. The command line, the help text, the OpenAPI
document and the HTML reference are all derived from these declarations,
and the derived artifacts are committed so a fresh clone needs no build
step. Five gates run in the pre-commit hook and reject the ways they
could drift apart: a route with no command, a handler reading a field
its cleaner would strip, a request parameter the core call does not
have, a generated file that is stale, and a command no test exercises.
The thirteen cover the shapes worth reviewing:
- no arguments, engine-local — `engine-status`, `engine-config`
- no arguments, reaching core — `local-users`, `fetch-login-messages`
- one named argument — `username-available`
- a body, and the session it establishes — `create-account`,
`login-with-password`, `logout`
- a positional path parameter — `object-get`, `object-delete`
- a held-open stream — `subscribe`
Path parameters are base58 identifiers and nothing else, because base64
wallet ids and free-text usernames contain `/` and cannot survive a URL
unescaped. Everything else is a named argument. A positional is declared
once as an ordinary field and the path is derived from it, so the two
cannot disagree.
`--fake` serves an in-process `makeFakeEdgeWorld`, which is what lets
the CLI tests run in a hook with no network, no server and no API key.
Core values with methods on them cannot cross JSON, so the engine keeps
them and hands back a handle: a staged transaction, a pending login, a
swap quote, a lobby. A handle carries its own TTL and is released when
the caller finishes with it, and a call that consumes one — approving a
swap — marks it in flight first, so a client that retries after its own
socket timeout is refused with `OBJECT_IN_USE` rather than spending
twice. A call that keeps its handle holds it the same way, so a
broadcast that outlives the TTL still returns its txid instead of
expiring between the send and the reply. Reading a handle returns a
projection, never the live object: serializing an `EdgeAccount` would
walk its `otpKey` and `recoveryKey` getters, and a swap quote reaches
both wallets and every token they know.
Responses are validated against the same cleaners that document them.
`checkResponse` runs each one and discards the cleaned value, since a
cleaner strips unknown keys and returning it would delete fields the
engine means to send; `EDGE_CLI_CHECK_RESPONSES` decides whether a
mismatch warns, fails the request, or is skipped. Drift shows up in the
log rather than reaching a caller unnoticed.
One engine serves one profile, and it claims the profile by creating its
run file exclusively before opening the data directory, so two cold
invocations cannot hold two `EdgeContext`s on one set of repos or unlink
each other's socket. The idle timer counts in-flight requests as well as
sessions and subscribers, so a cold login cannot be shut down underneath
itself. Everything the engine reads from disk — its run file, the
client's session file, the account's synced settings — goes through a
cleaner, and the bearer tokens in an OTP challenge are masked on their
way to a terminal while staying in the REST body the commands read them
from.
Account and session management, credentials, 2FA and vouchers, the data
store, keys and wallets, tokens, URIs, transactions and their export,
the staged spend path, swaps, exchange rates, and the `$internalStuff`
admin calls — a hundred and four `route({…})` declarations, each
carrying its own cleaners, error list and prose.
The five documentation gates hold across every one of them: the surface
matches, each response field carries prose, each call either matches its
`edge-core-js` signature or records why it differs, and all but four run
offline against the fake world. `npm run docs:api:gates` reports the
counts; they are not repeated here, because a number in a commit message
goes stale the first time a field is added.
Four cannot be exercised in a hook and say so: the two rates calls, swap
quotes and payment-protocol requests each reach a third-party API that
the fake world does not intercept. No suite covers them:
`test:cli:network` runs the one-shot, CAPTCHA and edge-login suites, and
none of the three names any of the four. The coverage gate records that
as the reason it excuses them.
Where the API departs from `edge-core-js` it is recorded in `coreExtra`
with the reason — a wallet object that cannot cross HTTP as anything but
an id, the `to`/`amount` shorthand that expands into `spendTargets`,
engine-side paging and export on `get-transactions`. Anything not listed
there fails the build.
Where a value cannot be derived safely the call refuses rather than
guesses. `rates-usd-to-native` requires its `multiplier`, because this
route has no logged-in account to read a denomination from and an
assumed one returns a `nativeAmount` wrong by orders of magnitude.
`sign-bytes` rejects malformed base64 instead of signing whatever
decoded. Handles record the resolved `wallet.id`, so a wallet id and a
unique prefix of it name the same wallet on every step of a staged
spend.
The CLI is built from this repository but is not this repository: rollup inlines every module it reaches under `src/`, so a published package is the two bundles, the native HMAC addon, the CLI document as its README and `LICENSE`. Nothing about it needs the CLI to move to a workspace or a submodule, and the app's own `package.json` stays `private: true` — what is published is a separate manifest assembled in a temporary directory, so the app itself cannot reach npm by accident. `src/cli/npmMeta.ts` holds the decisions: the scoped name, the bin name, the licence, and the per-platform native packages, which become `optionalDependencies` once they exist. The version is not among them. The CLI ships in lockstep with the app, so the manifest takes it from the app's `package.json`: there is no second number to bump, and a published CLI says which app release it corresponds to. Lockstep costs one thing worth knowing before a release — npm will not replace an existing version, so a CLI-only fix goes out on the next app version bump rather than on its own. The dependency list is derived, not written down. `rollup.config.cli.mjs` externalises every key of the app's `dependencies` — its whole runtime set — so the bundles leave all of them as bare `require`s while needing fifteen, and anything the app does not declare is inlined instead. `buildCliManifest.ts` walks the module graph from both entry points, keeps the bare specifiers the app declares as dependencies, drops builtins and type-only imports, and treats the rest as bundled. Checked against the bundles' own `require` calls: fifteen declared, fifteen required, none missing and none spare. Hand-maintaining that list fails as an `npm install` that succeeds and a CLI that cannot resolve a module on first run, so `cli:manifest:check` gates it in `verify` and in CI — where, unlike the five documentation gates, `npm run prepare` does not regenerate it first. `publishCli.ts` builds, stages and publishes. With `edgeKey.json` it runs `build:cli:all`, so a build server needs that one file to produce a CLI with full native signing; without it the addon cannot be built, so publishing takes an explicit `--allow-unsigned` and the staged README says the build cannot sign. A dirty tree is refused, since the registry copy could not then be re-derived from any commit. `--dry-run` packs without publishing and `--out` stages for inspection. Verified end to end: the staged tarball is 924 kB over six files, and installed into an empty project the client answers `--help` and the engine boots and serves `engine-status`. Two things the exercise surfaced, neither fixed here. `uuid` is imported by `src/util/utils.ts` and declared nowhere, resolved only because other packages happen to depend on it; rollup inlines it, so the published CLI is unaffected, but the build rests on a transitive resolution. And installing the package costs 2.3 GB across 69,756 files, of which about 1.3 GB is React Native mobile binaries — iOS simulator slices and Android libraries for the privacy coins — reached through `edge-currency-accountbased`, which also brings `react-native` itself. A Node CLI can load none of it.
3.review-errors.2, with 3.engine-routes.1 and 3.engine-infra.2. Five request-position amount fields were declared `asString` and handed to biggystring, which throws a plain `Error` that `toErrorBody` has no arm for — so `spend --native-amount=abc` and `get-transactions --spam-threshold=abc` answered 500 on routes declaring 400. A fractional value was worse than an error: the UTXO plugin sums fees with biggystring but builds the output with `parseInt`, so "1.5" funded the fee math at 1.5 and paid out 1, under a field documented as the chain's smallest unit. `asIntegerString` already existed for exactly this and is now exported and used by `asSpendTarget`, the spend shorthand's `nativeAmount` and `amount`, `swap-quote`, `encode-uri` and `spamThreshold`, where zero stays legal. Response-position amounts keep `asString`: a transaction's `nativeAmount` is negative for a send, and core's own values are already valid. `asEdgeTxAction` dispatched through a plain object literal, so `actionType: "toString"` resolved `Object.prototype.toString` — not null, so the unknown-actionType guard was skipped and a string was returned as an `EdgeTxAction`; `"constructor"` handed back the unvalidated body. Both reached `wallet.saveTxAction`, where core dispatches `CURRENCY_WALLET_FILE_CHANGED` before its own uncleaner rejects. Guarded with the shared `hasOwn`, as `util/exchangeDenom.ts` already does. Seven offline checks cover the new rejections, including both prototype names.
3.review-state.6, and 3.review-state.7 with 3.harness-review.8.
`--save-export-prefs` was honoured on one branch of three: the
`mergeExportTxInfo` call sat inside `if (formats.includes('bitwave'))` and
again inside the explicit-account-id test, so
`--export-format=csv,qbo --save-export-prefs` answered `ok` and wrote
nothing. The caller asked for their format choice to be remembered and the
GUI export scene's switches were untouched. The write now runs once for
every format combination, still only when the flag is given — writing
unasked turned a one-off `--bitwave-account-id` into the user's saved id.
Passing an absent id is safe because `mergeExportTxInfo` reads each field
as `patch.x ?? prev?.x`, which is what keeps a saved bitwave id through a
csv-only save.
`limit`'s published description said "Defaults to 100" while
`DEFAULT_TX_LIMIT` is 99 — deliberately, so a page prices in one rates
request. The wrong number ships in `openapi.json`, the HTML reference and
`edge-cli help`, so a caller paging on the documented default walks
`offset` 0, 100, 200 and skips one transaction per page, which `total`
cannot reveal because it is the match count. The description cannot
interpolate the constant: `extractRoutes` reads it through the checker as a
string literal, and a template literal drops the description from the
reference altogether. A test holds the two in step instead.
3.engine-routes.3, 3.engine-routes.2, 3.harness-review.4 and 3.review-state.5 — four routes whose declared reach and real reach differed. The key-export calls resolved through `findWallet`, which searches `account.currencyWallets`: core builds that only from `activeWalletIds` and only for wallets whose api exists. So `all-keys` listed an archived wallet and `get-raw-private-key` answered `WALLET_NOT_FOUND` for the exact id it had just printed — on the disaster-recovery path a CLI key export is for. Core's `getRawPrivateKey`, `getDisplayPrivateKey`, `getRawPublicKey` and `listSplittableWalletTypes` all work off `allKeys`, so `findWalletId` resolves there, with the same prefix contract and the same errors. `change-wallet-states` validated nothing. Core treats an id it has never seen as new and writes a state file for it without complaint, so a typo or a prefix answered 204 while nothing changed, and left a bogus `Keys/<hash>.json` in the account repo to sync to every device. It was also the one wallet route that ignored the prefix contract `walletId` is documented with. Every key now resolves over `allKeys` first. `save-tx` was the only handle-advancing route setting neither `consuming` nor `hold`, so `OBJECT_IN_USE`, the sweeper's skip and a bulk release's bounded wait were all blind to it. A slow save overlapping a `broadcast-tx` for the same handle removed the record under the in-flight broadcast, which then answered `OBJECT_NOT_FOUND` after the funds had left. It runs under `consume` now, which also does the delete the handler did by hand. `object-get` and `object-delete` published a reach they lost: only `transaction` and `swap` are created with a `sessionId`, so `pendingLogin` and `lobby` can only answer `OBJECT_SESSION_MISMATCH` there. Both descriptions say so, and the two dead arms of `projectHandleValue` are explicit about being unreachable and point at `pendingSummary`, which is where a pending login is really projected — the reasoning worth keeping, since serialising one walks into `otpKey` and `recoveryKey` getters.
3.review-servers.4, and 3.review-state.3 with 3.harness-review.2.
`handleRequest` ran `idle.touch()` and `idle.beginRequest()` before
`checkTcpRequest`, so every rejected request pushed `idleShutdownAt` out by
a full `--idle-timeout`. Any other local process — the threat
`transportAuth.ts` names — could poll the port once a minute with no token
and keep the daemon resident indefinitely, holding its EdgeContext and
every plugin's polling open, which is the leak `idleShutdown.ts` exists to
bound. The comment three lines below already claimed this ordering ("a
caller that cannot authenticate learns nothing about this engine"); now it
is true. A rejection is also logged at `warn`, because it is the only sign
of a probe and nothing recorded it.
The auto-logout ticker skipped any session whose window was `0`, which made
that value a one-way latch: a session created while the setting said `0`
captured it and the ticker never looked again, so a user turning
auto-logout back on from their phone had no effect on a session the engine
was already holding, while `engine-sessions` kept reporting
`autoLogoutSeconds: 0` as though it were still their choice. The cost
argument behind the skip is sound — `isExpired` answers false for `0`
before it looks at a clock, and the read is a decrypt plus a parse with no
cache — so those sessions now re-read once a minute against the ticker's
fifteen seconds. A quarter of the reads, and a bound on the staleness of a
security control.
The test that asserted the skip now asserts the cadence, and a new one
turns auto-logout back on mid-session and expects the logout.
3.engine-infra.3, 3.harness-review.6 and the closing paragraph of 3.review-async.2 — one defect found three times. `cleanupStaleLock` required a listening socket as well as a live pid, and its own comment asserted that `sweepStaleProfiles` "skips the directory for the same reason". The sweep tested the pid alone. `process.kill(pid, 0)` succeeds for ever once the OS hands that pid to something else, so the sweep permanently skipped the directories it exists to clear: a SIGKILLed engine's `session.json` — a full-account bearer token, which `removeRunArtifacts` is specifically there to delete — survived in a profile nothing would revisit, because `testCliFake` derives its data directory from the pid and every run hashes fresh. That is the 428 directories with 235 session files the sweep was written for. Both callers now share one `isClaimLive`, so they cannot drift again: a claim is live when something answers on its socket, or when it is young enough to still be booting. `sweepStaleProfiles` becomes async for the probe, which is local to its one caller in the async startup. A new case covers the recycled pid directly — live pid, nothing listening, backdated past the boot grace — and asserts the session file goes.
Summary
edge-cli/ engine).develop.testModeconfig, etc.).Notes for reviewers
develop(config/keys, native HMAC, Node-safe splits, CLI). It is intentionally draft-style until dependencies / base strategy are finalized; there is nofuture!pseudo-merge in the history.publish:cli) is a placeholder until packaging/bin metadata is restored.edgeKey.json(build:cli:native); stub builds are refused.Test plan
npm run test:cli:node-safenpm run build:cli/npm run build:cli:native(withedgeKey.json)npm run test:cli:node-hmac(withedgeKey.json)npm run test:cli/npm run test:cli:edge-loginas applicablenpm test/tsc