diff --git a/products/governance-api/cd/overlays/production/backendpolicy.yaml b/products/governance-api/cd/overlays/production/backendpolicy.yaml index ed486f4c9..9c4fbba68 100644 --- a/products/governance-api/cd/overlays/production/backendpolicy.yaml +++ b/products/governance-api/cd/overlays/production/backendpolicy.yaml @@ -3,6 +3,10 @@ kind: GCPBackendPolicy metadata: name: governance-api spec: + default: + # Proposal creation fetches + pins the full gZIL holder balances snapshot (~30s). Raise the + # backend timeout above GKE's 30s default so the request is not killed with a 504. + timeoutSec: 90 targetRef: group: "" kind: Service diff --git a/products/governance-api/cd/overlays/staging/backendpolicy.yaml b/products/governance-api/cd/overlays/staging/backendpolicy.yaml index ed486f4c9..9c4fbba68 100644 --- a/products/governance-api/cd/overlays/staging/backendpolicy.yaml +++ b/products/governance-api/cd/overlays/staging/backendpolicy.yaml @@ -3,6 +3,10 @@ kind: GCPBackendPolicy metadata: name: governance-api spec: + default: + # Proposal creation fetches + pins the full gZIL holder balances snapshot (~30s). Raise the + # backend timeout above GKE's 30s default so the request is not killed with a 504. + timeoutSec: 90 targetRef: group: "" kind: Service diff --git a/products/governance-api/lib/routes/message.ts b/products/governance-api/lib/routes/message.ts index b3ac9d9b1..d18cb4f2c 100644 --- a/products/governance-api/lib/routes/message.ts +++ b/products/governance-api/lib/routes/message.ts @@ -244,7 +244,7 @@ message.post("/message", async (req, res) => { log.info({ token: base16Token, address: base16owner, userBalance }, "Zilliqa liquidity fetched"); - const _balance = new BN(userBalance); + const _balance = new BN(userBalance || "0"); const _minGZIL = new BN("30000000000000000"); if (msg.token == gZIL && _balance.lt(_minGZIL)) { diff --git a/products/governance-api/lib/zilliqa/custom-fetch.test.ts b/products/governance-api/lib/zilliqa/custom-fetch.test.ts new file mode 100644 index 000000000..9231dfeae --- /dev/null +++ b/products/governance-api/lib/zilliqa/custom-fetch.test.ts @@ -0,0 +1,89 @@ +// ABOUTME: Framework-free assertions for getLiquidity's submitter-scoped balance fetch. +// ABOUTME: Run with `node --require ts-node/register lib/zilliqa/custom-fetch.test.ts`. +import assert from "assert"; +import { blockchain } from "./custom-fetch"; + +// The Scilla ZRC2 `balances` map is keyed by lowercase 0x addresses (verified against +// api.zilliqa.com). getLiquidity must (a) fetch ONLY the submitter's entry — not the whole +// multi-MB map — and (b) read it back with a normalized lowercase-0x key. + +async function fullMapFetchTest() { + // The balances RPC MUST fetch the FULL holder map (index []): it is pinned to IPFS as the + // whole-electorate voter-scoring snapshot. Scoping it to the submitter collapses vote tallies + // to the submitter alone (regression C1). The submitter's own balance is still read back via a + // normalized lowercase-0x key (fixes a latent bech32 checksum mismatch). + const b: any = new blockchain(); + let sent: any; + b._send = async (batch: any[]) => { + sent = batch; + return [ + { result: { pools: {} } }, + { result: { balances: {} } }, + { result: { xpools: {} } }, + { result: { xbalances: {} } }, + { result: { balances: { "0xabc": "123", "0xdef": "999" } } }, + { result: { total_supply: "1000" } }, + ]; + }; + + // Mixed-case submitter input to prove read-key normalization. + const out = await b.getLiquidity("0xA845c1034CD077bd8D32be0447239c7E4be6cb21", "0xABC"); + + assert.deepStrictEqual( + sent[4].params[2], + [], + "balances must fetch the FULL map (index []) for the voter-scoring snapshot (guards C1)" + ); + assert.strictEqual(out.userBalance, "123", "submitter balance read via normalized lowercase key"); + assert.ok("0xdef" in out.balances, "full electorate snapshot retained for scoring"); + console.log("OK fullMapFetchTest"); +} + +async function nullResultTest() { + const b: any = new blockchain(); + b._send = async () => [ + { result: null }, + { result: null }, + { result: null }, + { result: null }, + { result: null }, + { result: null }, + ]; + const out = await b.getLiquidity("0xA845c1034CD077bd8D32be0447239c7E4be6cb21", "0xfe56"); + assert.strictEqual(out.userBalance, "0", "missing balance => '0' (no throw)"); + assert.strictEqual(out.totalSupply, "0", "missing total_supply => '0' (no throw)"); + console.log("OK nullResultTest"); +} + +async function lpHolderCreditedWithoutDirectBalance() { + // A submitter whose gZIL sits ONLY in ZilSwap liquidity (no direct balance entry) must still + // be credited via the seeded key. Without the seed this returns "0" and wrongly blocks them. + const b: any = new blockchain(); + const TOKEN = "0xa845c1034cd077bd8d32be0447239c7e4be6cb21"; + const OWNER = "0xabc"; // -> ownerKey "0xabc" + b._send = async () => [ + { result: { pools: { [TOKEN]: { arguments: ["zilReserve", "1000000"] } } } }, + { result: { balances: { [TOKEN]: { "0xabc": "50" } } } }, // owner = 100% of a 50-unit pool + { result: { xpools: {} } }, + { result: { xbalances: {} } }, + { result: { balances: {} } }, // NO direct token balance for the owner + { result: { total_supply: "999" } }, + ]; + const out = await b.getLiquidity(TOKEN, OWNER); + assert.strictEqual( + out.userBalance, + "1000000", + "LP-only holder must be credited their pool share via the seeded key" + ); + console.log("OK lpHolderCreditedWithoutDirectBalance"); +} + +(async () => { + await fullMapFetchTest(); + await nullResultTest(); + await lpHolderCreditedWithoutDirectBalance(); + console.log("ALL PASS"); +})().catch((e) => { + console.error("FAIL:", e.message); + process.exit(1); +}); diff --git a/products/governance-api/lib/zilliqa/custom-fetch.ts b/products/governance-api/lib/zilliqa/custom-fetch.ts index 3a3be7e78..53801e1c8 100644 --- a/products/governance-api/lib/zilliqa/custom-fetch.ts +++ b/products/governance-api/lib/zilliqa/custom-fetch.ts @@ -23,6 +23,15 @@ export class blockchain { private _zero = new BN(0); public async getLiquidity(token: string, address: string) { + // Normalise the submitter's key (lowercase 0x, matching Scilla map keys) for the gate + // lookup + LP seeding below. + // + // NOTE: the FULL holder map is fetched on purpose (index [] below). It is pinned to IPFS + // as the whole-electorate voter-scoring snapshot (governance-snapshot get-scores.ts reads + // proposal.balances[voter]). Do NOT scope this to [ownerKey]: that shrinks the snapshot + // and collapses vote tallies to the submitter alone. The resulting ~30s fetch is covered + // by the raised gateway/client timeouts. + const ownerKey = "0x" + this._toHex(address); const batch = [ { method: RPCMethod.GetSmartContractSubState, @@ -72,13 +81,19 @@ export class blockchain { logger.error({ err, error_code: "ZILLIQA_RPC_FAILED" }, "Zilliqa RPC call failed"); throw err; } - let tokenBalances = res[4]["result"][TokenFields.Balances]; - const totalSupply = res[5]["result"][TokenFields.TotalSupply]; + let tokenBalances = res[4]?.["result"]?.[TokenFields.Balances] ?? {}; + const totalSupply = res[5]?.["result"]?.[TokenFields.TotalSupply] ?? "0"; + + // Seed the submitter's key so the LP crediting below can augment it even when the user + // holds no *direct* token balance (their gZIL may sit only in ZilSwap/XCAD liquidity). + if (!(ownerKey in tokenBalances)) { + tokenBalances[ownerKey] = "0"; + } tokenBalances = this._parseZilSwap(res, token, tokenBalances); tokenBalances = this._parseXcad(res, token, tokenBalances); - const userBalance = tokenBalances[address]; + const userBalance = tokenBalances[ownerKey] ?? "0"; logger.info({ token, address, userBalance }, "Zilliqa liquidity fetched"); return { @@ -178,7 +193,7 @@ export class blockchain { } private _toHex(address: string) { - return String(address).replace("0x", "").toLowerCase(); + return String(address).replace(/^0x/i, "").toLowerCase(); } private async _send(batch: object[]) { diff --git a/products/governance-api/package.json b/products/governance-api/package.json index d85f45ff7..3b084a4b0 100644 --- a/products/governance-api/package.json +++ b/products/governance-api/package.json @@ -7,7 +7,8 @@ "db:migrate": "npx sequelize db:migrate", "db:seed": "npx sequelize db:seed:all", "db:create": "npx sequelize db:create", - "start": "node --require ts-node/register index.ts" + "start": "node --require ts-node/register index.ts", + "test": "node --require ts-node/register lib/zilliqa/custom-fetch.test.ts" }, "author": "Hicaru", "license": "MIT", diff --git a/products/governance-snapshot/package.json b/products/governance-snapshot/package.json index f1e573f79..15f41a0f3 100644 --- a/products/governance-snapshot/package.json +++ b/products/governance-snapshot/package.json @@ -5,7 +5,8 @@ "scripts": { "serve": "NODE_OPTIONS=--openssl-legacy-provider vue-cli-service serve", "build": "NODE_OPTIONS=--openssl-legacy-provider vue-cli-service build", - "lint": "vue-cli-service lint" + "lint": "vue-cli-service lint", + "test": "TS_NODE_TRANSPILE_ONLY=1 TS_NODE_SKIP_PROJECT=1 TS_NODE_COMPILER_OPTIONS='{\"module\":\"commonjs\",\"target\":\"es2019\",\"esModuleInterop\":true}' node --require ../governance-api/node_modules/ts-node/register src/helpers/client.test.ts" }, "dependencies": { "@zilliqa-js/zilliqa": "3.5.0", diff --git a/products/governance-snapshot/src/helpers/client.test.ts b/products/governance-snapshot/src/helpers/client.test.ts new file mode 100644 index 000000000..8603a87d7 --- /dev/null +++ b/products/governance-snapshot/src/helpers/client.test.ts @@ -0,0 +1,110 @@ +// ABOUTME: Framework-free assertions that client.request always settles (never hangs the UI). +// ABOUTME: Run via ts-node transpile-only; mocks window + fetch. See command in the fix PR. +import assert from "assert"; + +(global as any).window = { VUE_APP_HUB_URL: "http://hub.test" }; + +// eslint-disable-next-line @typescript-eslint/no-var-requires +import client from "./client"; + +async function nonJsonErrorDoesNotHang() { + // A gateway 504 returns an HTML body → res.json() throws. Must still reject, not hang. + (global as any).fetch = async () => ({ + ok: false, + status: 504, + json: async () => { + throw new SyntaxError("Unexpected token < in JSON"); + } + }); + let rejected = false; + let val: any; + try { + await client.request("message", { a: 1 }); + } catch (e) { + rejected = true; + val = e; + } + assert.strictEqual(rejected, true, "504 with non-JSON body must reject, not hang"); + assert.ok( + val && typeof val.error_description === "string", + "rejection must carry a usable error_description" + ); + console.log("OK nonJsonErrorDoesNotHang:", val.error_description); +} + +async function successResolves() { + (global as any).fetch = async () => ({ + ok: true, + status: 200, + json: async () => ({ ipfsHash: "Qm123" }) + }); + const out: any = await client.request("message", { a: 1 }); + assert.deepStrictEqual(out, { ipfsHash: "Qm123" }, "success resolves to parsed JSON"); + console.log("OK successResolves"); +} + +async function abortRejects() { + (global as any).fetch = async () => { + const e: any = new Error("aborted"); + e.name = "AbortError"; + throw e; + }; + let rejected = false; + let val: any; + try { + await client.request("spaces"); + } catch (e) { + rejected = true; + val = e; + } + assert.strictEqual(rejected, true, "abort/timeout must reject"); + assert.ok(val.error_description.includes("timed out"), "AbortError => timeout message"); + console.log("OK abortRejects:", val.error_description); +} + +async function errorWithJsonRejectsServerObject() { + // A normal 400 (e.g. MIN_BALANCE) carries JSON the UI shows; reject with the server's object. + (global as any).fetch = async () => ({ + ok: false, + status: 400, + json: async () => ({ + code: 11, + error_description: "You require 30 $gZIL or more to submit a proposal." + }) + }); + let val: any; + try { + await client.request("message", { a: 1 }); + } catch (e) { + val = e; + } + assert.strictEqual(val.code, 11, "rejects with the server's JSON error object"); + assert.ok(val.error_description.includes("gZIL"), "preserves server error_description"); + console.log("OK errorWithJsonRejectsServerObject"); +} + +async function successEmptyBodyResolves() { + // A 2xx with a non-JSON/empty body must resolve (to undefined), never hang or throw. + (global as any).fetch = async () => ({ + ok: true, + status: 200, + json: async () => { + throw new SyntaxError("empty body"); + } + }); + const out = await client.request("spaces"); + assert.strictEqual(out, undefined, "non-JSON 2xx resolves to undefined"); + console.log("OK successEmptyBodyResolves"); +} + +(async () => { + await nonJsonErrorDoesNotHang(); + await successResolves(); + await abortRejects(); + await errorWithJsonRejectsServerObject(); + await successEmptyBodyResolves(); + console.log("ALL PASS"); +})().catch((e) => { + console.error("FAIL:", e.message); + process.exit(1); +}); diff --git a/products/governance-snapshot/src/helpers/client.ts b/products/governance-snapshot/src/helpers/client.ts index 0312a4aed..d28ce58b5 100644 --- a/products/governance-snapshot/src/helpers/client.ts +++ b/products/governance-snapshot/src/helpers/client.ts @@ -1,25 +1,57 @@ +// ABOUTME: Thin fetch wrapper for the governance-api. Guarantees the returned promise always +// ABOUTME: settles (timeout + defensive error parsing) so the UI never hangs on a 504/non-JSON. + +// Proposal creation pins the full gZIL holder snapshot (~30s); the gateway backend timeout is +// raised to 90s to match, so keep this client ceiling just above it to surface a real 504 +// instead of aborting first. It is a safety net against an indefinitely hung backend. +const REQUEST_TIMEOUT_MS = 95000; + class Client { - request(command, body?) { + async request(command, body?) { const url = `${window['VUE_APP_HUB_URL']}/api/${command}`; - let init; + const init: any = {}; if (body) { - init = { - method: 'POST', - headers: { - Accept: 'application/json', - 'Content-Type': 'application/json' - }, - body: JSON.stringify(body) + init.method = 'POST'; + init.headers = { + Accept: 'application/json', + 'Content-Type': 'application/json' }; + init.body = JSON.stringify(body); + } + + // Bound the request so a hung/slow backend can never leave the caller waiting forever. + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), REQUEST_TIMEOUT_MS); + init.signal = controller.signal; + + let res; + try { + res = await fetch(url, init); + } catch (e) { + // Network failure, CORS block, or abort (timeout). Reject with a usable shape. + const aborted = e && e.name === 'AbortError'; + return Promise.reject({ + error_description: aborted + ? 'Request timed out. Please try again.' + : 'Network error. Please try again.' + }); + } finally { + clearTimeout(timer); } - return new Promise((resolve, reject) => { - fetch(url, init) - .then(res => { - if (res.ok) return resolve(res.json()); - throw res; - }) - .catch(e => e.json().then(json => reject(json))); - }); + + // Error responses are frequently NOT JSON (a gateway 504 returns an HTML page), so parse + // defensively instead of letting `res.json()` reject and strand the outer promise. + const payload = await res.json().catch(() => undefined); + + if (res.ok) { + return payload; + } + + return Promise.reject( + payload && typeof payload === 'object' + ? payload + : { error_description: `Request failed (HTTP ${res.status}).` } + ); } }