fix(dev): make the documented local quick start work - #4398
Conversation
The quick start in the README (cp .env.local.example .env, docker compose up -d, npm run setup) currently fails on a clean checkout. Two independent causes: 1. The config requires REALUNIT_W2W_GAS_LOW_BALANCE_THRESHOLD and throws when it is unset, but the variable is missing from .env.local.example and is not generated by the setup script. The API compiles fine and then dies on boot with "Missing REALUNIT_W2W_GAS_LOW_BALANCE_THRESHOLD". The full .env.example already carries the variable with the same value, only the local template was missed. W2W transfers are inactive locally (DISABLED_PROCESSES=*), so the value only has to satisfy the check. 2. setup.js never loaded the .env it instructs the developer to create, so its own database checks fell back to the defaults in dbConfig(). With a customised SQL_* (for example a different port because 5432 is already in use) the script queried a different database than the API wrote to and aborted with the misleading "Database not ready after timeout" while the API was healthy. Other scripts in this repository already load dotenv the same way. Loading the .env before the safety checks also means ENVIRONMENT and SQL_HOST are now evaluated against the actual configuration instead of undefined defaults, which makes the existing production guard effective rather than weaker.
Drop the dotenv change from this pull request and correct the comment. Loading .env in setup.js turned out to be the wrong scope here. The safety check allows remote hosts (/^sql-dfx-api-loc/i and /loc.*\.database\.windows\.net/i), while dbConfig() previously never saw SQL_HOST and therefore always worked on localhost. Reading .env would have let setAdminRole() and seedDepositAddresses() write to a remote database for the first time - a weaker guarantee than before. It also only solved half the problem, because docker-compose.yml and the script's API_URL keep their hard-coded port and would still not follow a customised .env, and dotenv is not a direct dependency of this package. Making the setup script configurable is a separate change that has to cover the compose file, the API URL and a tightened host allow-list together. This pull request stays on the one defect that blocks every developer. The comment justified the value with DISABLED_PROCESSES=*, which is wrong: that setting only suppresses cron processes, and the threshold is read in the HTTP path (realunit.service.ts). The real reason the value is inert locally is that assertW2wGasWalletFunded() rejects while the W2W wallet credentials are unset, before the threshold is ever compared.
The value is destructured before the credential check; only the comparison happens after it.
|
Completed 3 review passes before this reached zero findings. Pass 1 removed a second change that had been bundled in: making Pass 1 also corrected the justification in the comment: Pass 2 tightened the same comment to say the threshold is compared rather than read after the credential check — it is destructured before it. Pass 3 came back clean on both dimensions. Final diff is 7 lines in one file. |
Problem
The quick start documented in the README fails on a clean checkout:
ConfigurationrequiresREALUNIT_W2W_GAS_LOW_BALANCE_THRESHOLDand throws when it is unset.The variable is absent from
.env.local.exampleand is not generated byscripts/setup.js,so the API compiles cleanly (
Found 0 errors) and then dies during boot:.env.example:342already carries the variable with the same value — only the local templatewas missed when the guard was introduced.
Change
One entry in
.env.local.example, with a comment explaining why the value is inert locally:assertW2wGasWalletFunded()(src/subdomains/supporting/realunit/realunit.service.ts:3234)rejects while
REALUNIT_W2W_GAS_WALLET_PRIVATE_KEY/_ADDRESSare unset, before thethreshold is ever compared. So locally the value only has to satisfy the boot check.
Scope
An earlier revision of this branch also made
scripts/setup.jsload.env, so that acustomised
SQL_*would apply to the script's own database checks. That was dropped again:/^sql-dfx-api-loc/i,/loc.*\.database\.windows\.net/i), whiledbConfig()previously never sawSQL_HOSTandtherefore always operated on localhost. Reading
.envwould have letsetAdminRole()andseedDepositAddresses()write to a remote database for the first time.docker-compose.ymland the script'sAPI_URLkeep their hard-coded values and would still ignore a customised
.env.dotenvis not a direct dependency of this package.Making the local setup genuinely configurable needs the compose file, the API URL and a
tightened host allow-list changed together. This pull request stays on the single defect that
blocks every developer.
Verification
npm run setupcompletes end to end — database ready, admin registered and promoted,5 deposit addresses seeded;
GET /v1/assetreturns 200 against the seeded database(229 assets, 250 countries, 24 fiats).
npm run format:checkpasses. The changed path is outside thelintandformatglobs(
{src,apps,libs,test}).