From 10e947149bf70281d77b7f1fb7b3f9c3253e4bf6 Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Sat, 17 Oct 2026 16:29:04 +0100 Subject: [PATCH 1/4] feat(monitoring): SLO alert rules, alertmanager routing, and metrics for alerts (#1622) - Prometheus rules: API 5xx rate, p95 latency, DB pool utilisation, RPC probe failure rate - Alertmanager: PagerDuty for critical, webhook for warnings; secrets read from files - Runbook per alert, linked from alert annotations - Backend: cumulative request-duration histogram, probe counters, DB pool gauge --- backend/src/db/deepReadiness.ts | 3 + backend/src/db/healthMonitor.ts | 2 + backend/src/metrics/MetricsCollector.ts | 71 +++++++++++++++++ backend/src/metrics/MetricsExporter.ts | 66 +++++++++++++++- backend/src/metrics/dbPoolMetrics.ts | 23 ++++++ backend/tests/metricsSlo.test.ts | 101 ++++++++++++++++++++++++ monitoring/alertmanager/config.yml | 44 +++++++++++ monitoring/prometheus/alert.rules.yml | 85 ++++++++++++++++++++ monitoring/runbooks.md | 39 +++++++++ 9 files changed, 432 insertions(+), 2 deletions(-) create mode 100644 backend/src/metrics/dbPoolMetrics.ts create mode 100644 backend/tests/metricsSlo.test.ts create mode 100644 monitoring/alertmanager/config.yml create mode 100644 monitoring/prometheus/alert.rules.yml create mode 100644 monitoring/runbooks.md diff --git a/backend/src/db/deepReadiness.ts b/backend/src/db/deepReadiness.ts index 425bde661..f26b91232 100644 --- a/backend/src/db/deepReadiness.ts +++ b/backend/src/db/deepReadiness.ts @@ -19,6 +19,7 @@ import { Horizon } from '@stellar/stellar-sdk'; import prisma from '../db/index.js'; import redisClient from '../cache/RedisClient.js'; import { HORIZON_URL, SOROBAN_RPC_URL } from '../config/rpcConfig.js'; +import metricsCollector from '../metrics/MetricsCollector.js'; import logger from '../utils/logger.js'; const DEFAULT_PROBE_TIMEOUT_MS = 3000; @@ -128,12 +129,14 @@ async function probeSorobanRpc(timeout: number): Promise { timeout ); const latest = ledgers.records?.[0]?.sequence ?? null; + metricsCollector.recordProbe('horizon_ledger', latest != null); return { status: latest != null ? 'ready' : 'unavailable', latencyMs: Date.now() - start, detail: { latestLedgerHeight: latest }, }; } catch (error) { + metricsCollector.recordProbe('horizon_ledger', false); logger.error('Deep readiness: Soroban/Horizon probe failed', error); return { status: 'unavailable', diff --git a/backend/src/db/healthMonitor.ts b/backend/src/db/healthMonitor.ts index c5199e618..f70c2229c 100644 --- a/backend/src/db/healthMonitor.ts +++ b/backend/src/db/healthMonitor.ts @@ -1,5 +1,6 @@ import prisma from '../db/index.js'; import logger from '../utils/logger.js'; +import { setDbPoolUtilizationPct } from '../metrics/dbPoolMetrics.js'; export interface DbHealthStatus { status: 'healthy' | 'degraded' | 'unhealthy'; @@ -60,6 +61,7 @@ export async function checkDbHealth(): Promise { total: poolMax, utilizationPct, }; + setDbPoolUtilizationPct(utilizationPct); if (latencyMs > LATENCY_CRITICAL_MS) { alerts.push(`Critical: DB query latency ${latencyMs}ms exceeds ${LATENCY_CRITICAL_MS}ms`); diff --git a/backend/src/metrics/MetricsCollector.ts b/backend/src/metrics/MetricsCollector.ts index e2c59ceb4..18efb400f 100644 --- a/backend/src/metrics/MetricsCollector.ts +++ b/backend/src/metrics/MetricsCollector.ts @@ -17,6 +17,27 @@ import logger from '../utils/logger.js'; +// ─── Histogram + probe types ────────────────────────────────────────────────── + +/** + * Upper bounds (ms) for the cumulative request-duration histogram. Chosen so + * the 1500 ms SLO threshold is an exact bucket boundary, which keeps the p95 + * alert accurate. + */ +export const DURATION_BUCKETS_MS: readonly number[] = [50, 100, 250, 500, 1000, 1500, 2500, 5000]; + +export interface DurationHistogramSnapshot { + /** Cumulative counts: each bucket counts requests with duration <= le. */ + buckets: Array<{ le: number; count: number }>; + count: number; + sumMs: number; +} + +export interface ProbeCounts { + ok: number; + failed: number; +} + // ─── Types ──────────────────────────────────────────────────────────────────── export interface PerformanceMetric { @@ -86,6 +107,13 @@ export class MetricsCollector { private errorMetrics: ErrorMetric[] = []; private businessMetrics: BusinessMetric[] = []; + // Cumulative counters that survive ring-buffer trimming, so histogram and + // probe rates stay correct over long-running processes. + private durationBucketCounts: number[] = DURATION_BUCKETS_MS.map(() => 0); + private durationCount = 0; + private durationSumMs = 0; + private probeCounts = new Map(); + /** * Maximum number of raw metric entries kept in memory per category. * Older entries are dropped (ring-buffer style) to bound memory usage. @@ -119,6 +147,14 @@ export class MetricsCollector { this.performanceMetrics.push(metric); this.trim(this.performanceMetrics); + if (Number.isFinite(durationMs) && durationMs >= 0) { + this.durationCount += 1; + this.durationSumMs += durationMs; + DURATION_BUCKETS_MS.forEach((le, i) => { + if (durationMs <= le) this.durationBucketCounts[i] = (this.durationBucketCounts[i] ?? 0) + 1; + }); + } + // Log slow requests (>1 s) so they surface in existing log pipelines. if (durationMs > 1000) { logger.warn(`Slow request: ${method} ${route} took ${durationMs}ms (status ${statusCode})`); @@ -259,10 +295,45 @@ export class MetricsCollector { /** * Reset all collected metrics (useful in tests or after a scheduled flush). */ + /** + * Cumulative request-duration histogram since process start (or last reset). + */ + getDurationHistogram(): DurationHistogramSnapshot { + return { + buckets: DURATION_BUCKETS_MS.map((le, i) => ({ le, count: this.durationBucketCounts[i] ?? 0 })), + count: this.durationCount, + sumMs: this.durationSumMs, + }; + } + + /** + * Record the outcome of a periodic dependency probe (e.g. the deep readiness + * ledger check). `probe` is a low-cardinality label such as "horizon_ledger". + */ + recordProbe(probe: string, ok: boolean): void { + const counts = this.probeCounts.get(probe) ?? { ok: 0, failed: 0 }; + if (ok) counts.ok += 1; + else counts.failed += 1; + this.probeCounts.set(probe, counts); + } + + /** Cumulative probe outcomes keyed by probe name. */ + getProbeCounts(): Record { + const out: Record = {}; + this.probeCounts.forEach((value, key) => { + out[key] = { ok: value.ok, failed: value.failed }; + }); + return out; + } + reset(): void { this.performanceMetrics = []; this.errorMetrics = []; this.businessMetrics = []; + this.durationBucketCounts = DURATION_BUCKETS_MS.map(() => 0); + this.durationCount = 0; + this.durationSumMs = 0; + this.probeCounts = new Map(); logger.info('[Metrics] All metrics reset'); } diff --git a/backend/src/metrics/MetricsExporter.ts b/backend/src/metrics/MetricsExporter.ts index 12f93dbb5..16d5f5fbe 100644 --- a/backend/src/metrics/MetricsExporter.ts +++ b/backend/src/metrics/MetricsExporter.ts @@ -11,10 +11,15 @@ * stays bounded. */ -import metricsCollector, { type MetricsSummary } from './MetricsCollector.js'; +import metricsCollector, { + type DurationHistogramSnapshot, + type MetricsSummary, + type ProbeCounts, +} from './MetricsCollector.js'; import workerRegistry, { type WorkerStatus } from './WorkerRegistry.js'; import cacheService from '../cache/CacheService.js'; import redisClient from '../cache/RedisClient.js'; +import { getDbPoolUtilizationPct } from './dbPoolMetrics.js'; /** Snapshot schema version. Bump on a breaking field change. */ export const METRICS_SCHEMA_VERSION = '1'; @@ -56,6 +61,12 @@ export interface MetricsSnapshot { business: { eventsTotal: number; eventsByName: Record }; workers: WorkerStatus[]; system: { uptimeSeconds: number; memoryResidentBytes: number; cpuUserSeconds: number }; + /** Cumulative request-duration histogram (optional for hand-built snapshots). */ + durationHistogram?: DurationHistogramSnapshot; + /** Dependency probe outcomes keyed by probe name. */ + probes?: Record; + /** Last recorded DB pool utilisation (0-100), or null if never measured. */ + dbPoolUtilizationPct?: number | null; } /** Segments that look like identifiers rather than route names. */ @@ -147,9 +158,28 @@ export function buildMetricsSnapshot( memoryResidentBytes: Math.round(summary.system.memoryUsageMB * 1024 * 1024), cpuUserSeconds: Number((summary.system.cpuUserMs / 1000).toFixed(3)), }, + durationHistogram: metricsCollector.getDurationHistogram(), + probes: metricsCollector.getProbeCounts(), + dbPoolUtilizationPct: getDbPoolUtilizationPct(), }; } +/** + * Render a Prometheus histogram (cumulative `_bucket` lines plus `_sum` and + * `_count`). The generic renderFamily helper only supports counters and + * gauges, so histograms are rendered here. + */ +function renderHistogram(name: string, help: string, h: DurationHistogramSnapshot): string { + const lines = [`# HELP ${name} ${help}`, `# TYPE ${name} histogram`]; + for (const bucket of h.buckets) { + lines.push(`${name}_bucket{le="${bucket.le}"} ${bucket.count}`); + } + lines.push(`${name}_bucket{le="+Inf"} ${h.count}`); + lines.push(`${name}_sum ${h.sumMs}`); + lines.push(`${name}_count ${h.count}`); + return lines.join('\n'); +} + // ─── Prometheus text exposition ─────────────────────────────────────────────── /** Escape a label value per the Prometheus exposition format. */ @@ -304,7 +334,39 @@ export function renderPrometheus(snapshot: MetricsSnapshot = buildMetricsSnapsho }, ]; - return `${families.map(renderFamily).join('\n')}\n`; + if (snapshot.dbPoolUtilizationPct !== undefined && snapshot.dbPoolUtilizationPct !== null) { + families.push({ + name: `${PREFIX}_db_pool_utilization_percent`, + help: 'Database connection pool utilisation from the last health check, 0-100.', + type: 'gauge', + samples: [{ value: snapshot.dbPoolUtilizationPct }], + }); + } + + if (snapshot.probes && Object.keys(snapshot.probes).length > 0) { + families.push({ + name: `${PREFIX}_external_probe_total`, + help: 'Dependency probe outcomes since process start, by probe and outcome.', + type: 'counter', + samples: Object.entries(snapshot.probes).flatMap(([probe, counts]) => [ + { labels: { probe, outcome: 'ok' }, value: counts.ok }, + { labels: { probe, outcome: 'failed' }, value: counts.failed }, + ]), + }); + } + + const rendered = families.map(renderFamily); + if (snapshot.durationHistogram) { + rendered.push( + renderHistogram( + `${PREFIX}_http_request_duration_milliseconds`, + 'HTTP request duration histogram in milliseconds, cumulative since process start.', + snapshot.durationHistogram + ) + ); + } + + return `${rendered.join('\n')}\n`; } /** Content type expected by Prometheus-compatible scrapers. */ diff --git a/backend/src/metrics/dbPoolMetrics.ts b/backend/src/metrics/dbPoolMetrics.ts new file mode 100644 index 000000000..ef4c96b54 --- /dev/null +++ b/backend/src/metrics/dbPoolMetrics.ts @@ -0,0 +1,23 @@ +/** + * Last-known database connection pool utilisation, written by the DB health + * check and read by the Prometheus exporter. Kept in its own module so the + * exporter does not import the health check (avoids a circular dependency). + */ + +let poolUtilizationPct: number | null = null; + +/** Record the latest pool utilisation (0-100). Non-finite values are ignored. */ +export function setDbPoolUtilizationPct(value: number): void { + if (!Number.isFinite(value)) return; + poolUtilizationPct = Math.min(100, Math.max(0, value)); +} + +/** Latest recorded pool utilisation, or null if no health check has run yet. */ +export function getDbPoolUtilizationPct(): number | null { + return poolUtilizationPct; +} + +/** Test helper: forget the recorded value. */ +export function resetDbPoolUtilizationPct(): void { + poolUtilizationPct = null; +} diff --git a/backend/tests/metricsSlo.test.ts b/backend/tests/metricsSlo.test.ts new file mode 100644 index 000000000..aaf9ae9b0 --- /dev/null +++ b/backend/tests/metricsSlo.test.ts @@ -0,0 +1,101 @@ +/** + * SLO metric exposure: request-duration histogram (for p95 alerts), dependency + * probe counters (for the RPC failure-rate alert) and DB pool utilisation + * (for the pool alert). These feed monitoring/prometheus/alert.rules.yml. + */ + +import metricsCollector, { DURATION_BUCKETS_MS } from '../src/metrics/MetricsCollector.js'; +import { renderPrometheus } from '../src/metrics/MetricsExporter.js'; +import { + getDbPoolUtilizationPct, + resetDbPoolUtilizationPct, + setDbPoolUtilizationPct, +} from '../src/metrics/dbPoolMetrics.js'; + +describe('request duration histogram', () => { + beforeEach(() => { + metricsCollector.reset(); + }); + + it('counts requests cumulatively per bucket upper bound', () => { + metricsCollector.recordRequest('GET', '/a', 40, 200); // <= 50 + metricsCollector.recordRequest('GET', '/a', 600, 200); // <= 1000 + metricsCollector.recordRequest('GET', '/a', 1600, 200); // <= 2500 + + const h = metricsCollector.getDurationHistogram(); + const byLe = Object.fromEntries(h.buckets.map((b) => [b.le, b.count])); + + expect(h.count).toBe(3); + expect(h.sumMs).toBe(2240); + expect(byLe[50]).toBe(1); + expect(byLe[500]).toBe(1); + expect(byLe[1000]).toBe(2); + expect(byLe[1500]).toBe(2); + expect(byLe[2500]).toBe(3); + expect(h.buckets.map((b) => b.le)).toEqual([...DURATION_BUCKETS_MS]); + }); + + it('keeps the 1500 ms SLO threshold as an exact bucket boundary', () => { + expect(DURATION_BUCKETS_MS).toContain(1500); + }); + + it('renders histogram lines with +Inf, _sum and _count', () => { + metricsCollector.recordRequest('GET', '/a', 120, 200); + + const text = renderPrometheus(); + + expect(text).toContain('# TYPE w3sl_http_request_duration_milliseconds histogram'); + expect(text).toContain('w3sl_http_request_duration_milliseconds_bucket{le="250"} 1'); + expect(text).toContain('w3sl_http_request_duration_milliseconds_bucket{le="+Inf"} 1'); + expect(text).toContain('w3sl_http_request_duration_milliseconds_sum 120'); + expect(text).toContain('w3sl_http_request_duration_milliseconds_count 1'); + }); + + it('is cleared by reset()', () => { + metricsCollector.recordRequest('GET', '/a', 10, 200); + metricsCollector.reset(); + expect(metricsCollector.getDurationHistogram().count).toBe(0); + }); +}); + +describe('dependency probe counters', () => { + beforeEach(() => { + metricsCollector.reset(); + }); + + it('counts ok and failed probe outcomes per probe', () => { + metricsCollector.recordProbe('horizon_ledger', true); + metricsCollector.recordProbe('horizon_ledger', true); + metricsCollector.recordProbe('horizon_ledger', false); + + expect(metricsCollector.getProbeCounts()).toEqual({ + horizon_ledger: { ok: 2, failed: 1 }, + }); + + const text = renderPrometheus(); + expect(text).toContain('w3sl_external_probe_total{probe="horizon_ledger",outcome="ok"} 2'); + expect(text).toContain('w3sl_external_probe_total{probe="horizon_ledger",outcome="failed"} 1'); + }); +}); + +describe('DB pool utilisation gauge', () => { + afterEach(() => { + resetDbPoolUtilizationPct(); + }); + + it('is omitted until a health check has recorded a value', () => { + expect(getDbPoolUtilizationPct()).toBeNull(); + expect(renderPrometheus()).not.toContain('w3sl_db_pool_utilization_percent'); + }); + + it('renders the last recorded utilisation and clamps to 0-100', () => { + setDbPoolUtilizationPct(87); + expect(renderPrometheus()).toContain('w3sl_db_pool_utilization_percent 87'); + + setDbPoolUtilizationPct(250); + expect(getDbPoolUtilizationPct()).toBe(100); + + setDbPoolUtilizationPct(Number.NaN); + expect(getDbPoolUtilizationPct()).toBe(100); + }); +}); diff --git a/monitoring/alertmanager/config.yml b/monitoring/alertmanager/config.yml new file mode 100644 index 000000000..325fb140a --- /dev/null +++ b/monitoring/alertmanager/config.yml @@ -0,0 +1,44 @@ +# Alertmanager routing for Web3 Student Lab (BE-050, #1622). +# +# Secrets are read from files so they are never committed. Mount them into the +# Alertmanager container at /etc/alertmanager/secrets/ (for example from a +# Kubernetes Secret). Requires Alertmanager >= 0.27 for *_file options. +# +# Critical alerts page PagerDuty; warnings go to a webhook (for example a +# Discord or Slack relay). Both receivers get a resolved notification. + +global: + resolve_timeout: 5m + +route: + receiver: warnings-webhook + group_by: ["alertname", "service"] + group_wait: 10s + group_interval: 1m + repeat_interval: 4h + routes: + - matchers: + - severity="critical" + receiver: pagerduty-critical + group_wait: 10s + repeat_interval: 1h + - matchers: + - severity="warning" + receiver: warnings-webhook + +receivers: + - name: pagerduty-critical + pagerduty_configs: + - routing_key_file: /etc/alertmanager/secrets/pagerduty_routing_key + severity: "{{ .CommonLabels.severity }}" + description: "{{ .CommonAnnotations.summary }}" + send_resolved: true + details: + runbook: "{{ .CommonAnnotations.runbook_url }}" + diagnostic_query: "{{ .CommonAnnotations.diagnostic_query }}" + + - name: warnings-webhook + webhook_configs: + - url_file: /etc/alertmanager/secrets/warning_webhook_url + send_resolved: true + max_alerts: 20 diff --git a/monitoring/prometheus/alert.rules.yml b/monitoring/prometheus/alert.rules.yml new file mode 100644 index 000000000..f1315def4 --- /dev/null +++ b/monitoring/prometheus/alert.rules.yml @@ -0,0 +1,85 @@ +# Prometheus alerting rules for Web3 Student Lab SLOs (BE-050, #1622). +# +# Metric sources (backend/src/metrics): +# w3sl_http_responses_total{status_class} MetricsExporter +# w3sl_http_request_duration_milliseconds_bucket{le} MetricsCollector histogram +# w3sl_db_pool_utilization_percent dbPoolMetrics (set by DB health check) +# w3sl_external_probe_total{probe,outcome} deepReadiness probe outcomes +# +# Noise control: each ratio alert also requires a minimum request rate so a +# quiet service does not page on one failed request, and a `for:` duration so a +# single bad evaluation does not fire. Evaluation interval is 15s, so an alert +# fires roughly 45-60s after a sustained breach. + +groups: + - name: web3-student-lab-slo + interval: 15s + rules: + - alert: ApiErrorRateHigh + expr: | + ( + sum(rate(w3sl_http_responses_total{status_class="5xx"}[5m])) + / + clamp_min(sum(rate(w3sl_http_responses_total[5m])), 1e-9) + ) > 0.01 + and sum(rate(w3sl_http_responses_total[5m])) > 0.1 + for: 30s + labels: + severity: critical + service: backend + slo: api-error-rate + annotations: + summary: "API 5xx error rate above 1% for 30s" + description: "5xx responses are {{ $value | humanizePercentage }} of all responses over the last 5m." + runbook_url: "https://github.com/StellarDevHub/Web3-Student-Lab/blob/main/monitoring/runbooks.md#apierrorratehigh" + diagnostic_query: 'sum by (status_class) (rate(w3sl_http_responses_total[5m]))' + + - alert: ApiP95LatencyHigh + expr: | + histogram_quantile( + 0.95, + sum by (le) (rate(w3sl_http_request_duration_milliseconds_bucket[5m])) + ) > 1500 + and sum(rate(w3sl_http_request_duration_milliseconds_count[5m])) > 0.1 + for: 1m + labels: + severity: warning + service: backend + slo: api-p95-latency + annotations: + summary: "API p95 response time above 1500 ms" + description: "p95 latency is {{ $value | humanize }} ms over the last 5m." + runbook_url: "https://github.com/StellarDevHub/Web3-Student-Lab/blob/main/monitoring/runbooks.md#apip95latencyhigh" + diagnostic_query: 'histogram_quantile(0.95, sum by (le) (rate(w3sl_http_request_duration_milliseconds_bucket[5m])))' + + - alert: DbPoolUtilizationHigh + expr: w3sl_db_pool_utilization_percent > 85 + for: 30s + labels: + severity: critical + service: database + slo: db-pool-utilization + annotations: + summary: "Database connection pool above 85% utilisation" + description: "Pool utilisation is {{ $value }}% of DB_POOL_MAX (last health check)." + runbook_url: "https://github.com/StellarDevHub/Web3-Student-Lab/blob/main/monitoring/runbooks.md#dbpoolutilizationhigh" + diagnostic_query: 'w3sl_db_pool_utilization_percent' + + - alert: RpcProbeFailureRateHigh + expr: | + ( + sum(rate(w3sl_external_probe_total{probe="horizon_ledger",outcome="failed"}[5m])) + / + clamp_min(sum(rate(w3sl_external_probe_total{probe="horizon_ledger"}[5m])), 1e-9) + ) > 0.05 + and sum(rate(w3sl_external_probe_total{probe="horizon_ledger"}[5m])) > 0 + for: 1m + labels: + severity: warning + service: stellar-rpc + slo: rpc-failure-rate + annotations: + summary: "Stellar RPC readiness probe failing more than 5% of the time" + description: "Probe failure ratio is {{ $value | humanizePercentage }} over the last 5m." + runbook_url: "https://github.com/StellarDevHub/Web3-Student-Lab/blob/main/monitoring/runbooks.md#rpcprobefailureratehigh" + diagnostic_query: 'sum by (outcome) (rate(w3sl_external_probe_total{probe="horizon_ledger"}[5m]))' diff --git a/monitoring/runbooks.md b/monitoring/runbooks.md new file mode 100644 index 000000000..fec47e2dd --- /dev/null +++ b/monitoring/runbooks.md @@ -0,0 +1,39 @@ +# Monitoring runbooks + +One section per alert in `prometheus/alert.rules.yml`. Each alert's +`runbook_url` links to its heading here. + +## ApiErrorRateHigh + +5xx responses exceed 1% of all responses for 30 seconds (with at least 0.1 req/s traffic). + +1. Check the 5xx breakdown: `sum by (status_class) (rate(w3sl_http_responses_total[5m]))`. +2. Check worker and error types: `GET /api/v1/metrics/prometheus` and review `w3sl_errors_total` by `type`. +3. Check backend logs for `Unhandled error` entries and recent deploys. +4. If a dependency is down (see DB and RPC alerts), resolve that first. + +## ApiP95LatencyHigh + +p95 request duration exceeds 1500 ms over 5 minutes. + +1. Confirm with `histogram_quantile(0.95, sum by (le) (rate(w3sl_http_request_duration_milliseconds_bucket[5m])))`. +2. Compare with `w3sl_db_pool_utilization_percent`. Pool saturation is the usual cause. +3. Check slow routes in logs (requests over 1 s are logged as `Slow request`). + +## DbPoolUtilizationHigh + +Database pool utilisation is above 85% of `DB_POOL_MAX`. + +1. The value updates on each DB health check, so confirm checks are running. +2. Look for long-running queries: `SELECT pid, state, now() - query_start AS age, left(query, 120) FROM pg_stat_activity WHERE datname = current_database() ORDER BY age DESC;` +3. Check for a traffic burst, and raise `DB_POOL_MAX` only after confirming the database has headroom. + +## RpcProbeFailureRateHigh + +The deep-readiness ledger probe (`probe="horizon_ledger"`) fails more than 5% of the time. + +Note: this probe currently calls Horizon's ledgers endpoint, not Soroban RPC. + +1. Check the Horizon status page and `HORIZON_URL` in the environment. +2. Check outbound connectivity from the backend pod. +3. Check the probe failure count: `sum by (outcome) (rate(w3sl_external_probe_total{probe="horizon_ledger"}[5m]))`. From 1f686e193f4c8606c407f08aa567415a052c202a Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Sat, 17 Oct 2026 16:29:05 +0100 Subject: [PATCH 2/4] feat(certificate): soulbound certificate revocation, expiry, and verification (#1624) - transfer/transfer_from panic with NonTransferable - revoke publishes CertificateRevokedEvent; optional institution ed25519 revocation - Optional expires_at and verify_certificate returning Valid/Expired/Revoked/NotFound - Fix unclosed braces in certificate_nft lib.rs --- contracts/certificate/src/errors.rs | 2 + contracts/certificate/src/lib.rs | 210 +++++++++++++++++-- contracts/certificate/src/test.rs | 255 ++++++++++++++++++++++-- contracts/certificate_nft/src/badges.rs | 2 +- contracts/certificate_nft/src/lib.rs | 104 +++++----- contracts/certificate_nft/src/test.rs | 22 +- 6 files changed, 505 insertions(+), 90 deletions(-) diff --git a/contracts/certificate/src/errors.rs b/contracts/certificate/src/errors.rs index 11eebe126..2c5950910 100644 --- a/contracts/certificate/src/errors.rs +++ b/contracts/certificate/src/errors.rs @@ -17,6 +17,8 @@ pub enum CertificateError { Unauthorized = 101, /// 106 — Soulbound certificate cannot be transferred or delegated. SoulboundNonTransferable = 106, + /// 107 - Certificate transfer or delegation attempted; certificates are non-transferable. + NonTransferable = 107, /// 201 — Certificate record was not found in persistent storage. NotFound = 201, /// 202 — Certificate has already been revoked. diff --git a/contracts/certificate/src/lib.rs b/contracts/certificate/src/lib.rs index c9a2b2923..5565c8237 100644 --- a/contracts/certificate/src/lib.rs +++ b/contracts/certificate/src/lib.rs @@ -1,8 +1,8 @@ #![no_std] use soroban_sdk::{ - contract, contracterror, contractimpl, contracttype, panic_with_error, Address, Bytes, BytesN, - Env, String, Vec, + contract, contracterror, contractimpl, contracttype, panic_with_error, symbol_short, Address, + Bytes, BytesN, Env, String, Vec, }; #[contracttype] @@ -14,6 +14,17 @@ pub enum CertificateStatus { Expired, } +/// Result of `verify_certificate`. Exactly one state is returned for any +/// certificate, with `Revoked` taking precedence over `Expired`. +#[contracttype] +#[derive(Clone, Debug, PartialEq)] +pub enum VerificationStatus { + Valid, + Expired, + Revoked, + NotFound, +} + #[contracttype] #[derive(Clone, Debug)] pub struct CertificateRecord { @@ -25,12 +36,18 @@ pub struct CertificateRecord { pub status: CertificateStatus, pub revocation_reason: String, pub previous_cert_id: BytesN<32>, + /// Hash of the off-chain metadata document, verifiable via + /// `verify_content_hash`. pub content_hash: BytesN<32>, + /// URI pointing at the off-chain metadata document (e.g. `ipfs://...`). + pub metadata_uri: String, + /// Optional expiration timestamp (ledger seconds). `None` never expires. + pub expires_at: Option, /// SEP-0029 secondary-sale royalty cut, in basis points (1/100 of a /// percent). 250 == 2.5%. Capped at `MAX_ROYALTY_BPS`. pub royalty_bps: u32, /// Certificates are soulbound (non-transferable) by default. This flag - /// is set once at issuance and can never be cleared — it is an + /// is set once at issuance and can never be cleared; it is an /// immutable revocation of transferability, not a toggle. pub soulbound: bool, } @@ -51,6 +68,16 @@ pub struct RevocationAuditLog { pub action: String, } +/// Structured event published whenever a certificate is revoked. +#[contracttype] +#[derive(Clone, Debug)] +pub struct CertificateRevokedEvent { + pub cert_id: BytesN<32>, + pub issuer: Address, + pub reason_code: String, + pub timestamp: u64, +} + #[contracttype] #[derive(Clone, Debug)] pub struct MerkleCohortRoot { @@ -70,12 +97,29 @@ impl CertificateContract { env.storage().instance().set(&DataKey::NextCertId, &0u64); } + /// Registers the institutional ed25519 public key whose signatures + /// authorize revocation via `revoke_certificate_with_institution_sig`. + /// Admin only. + pub fn set_institution_key(env: Env, public_key: BytesN<32>) { + let admin: Address = env + .storage() + .instance() + .get(&DataKey::Admin) + .unwrap_or_else(|| panic_with_error!(&env, Error::Unauthorized)); + admin.require_auth(); + env.storage() + .instance() + .set(&DataKey::InstitutionKey, &public_key); + } + pub fn issue_certificate( env: Env, owner: Address, issuer: Address, course_id: String, content_hash: BytesN<32>, + metadata_uri: String, + expires_at: Option, ) -> BytesN<32> { issuer.require_auth(); @@ -98,6 +142,8 @@ impl CertificateContract { revocation_reason: String::from_str(&env, ""), previous_cert_id: BytesN::from_array(&env, &[0u8; 32]), content_hash, + metadata_uri, + expires_at, royalty_bps: 0, // Immutable soulbound flag: certificates are non-transferable // from the moment they are issued. @@ -128,6 +174,7 @@ impl CertificateContract { cert_id_bytes } + /// Revokes a certificate. Requires the issuer's authorization. pub fn revoke_certificate(env: Env, cert_id: BytesN<32>, reason_code: String) { let mut record: CertificateRecord = env .storage() @@ -140,25 +187,48 @@ impl CertificateContract { // Enforce native Soroban authentication: only issuer may revoke record.issuer.require_auth(); - record.status = CertificateStatus::Revoked; - record.revocation_reason = reason_code.clone(); - env.storage() + Self::apply_revocation(&env, &mut record, reason_code); + } + + /// Revokes a certificate authorized by an institutional ed25519 + /// signature instead of issuer auth. The signature must be over the + /// message `cert_id (32 bytes) || reason_code (UTF-8 bytes)` made with + /// the key registered via `set_institution_key`. Traps if the + /// signature is invalid or no institution key is registered. + pub fn revoke_certificate_with_institution_sig( + env: Env, + cert_id: BytesN<32>, + reason_code: String, + signature: BytesN<64>, + ) { + let institution_key: BytesN<32> = env + .storage() + .instance() + .get(&DataKey::InstitutionKey) + .unwrap_or_else(|| panic_with_error!(&env, Error::Unauthorized)); + + let mut record: CertificateRecord = env + .storage() .persistent() - .set(&DataKey::Certificate(cert_id.clone()), &record); + .get(&DataKey::Certificate(cert_id.clone())) + .unwrap_or_else(|| panic_with_error!(&env, Error::NotFound)); + if record.status == CertificateStatus::Revoked { + panic_with_error!(&env, Error::AlreadyRevoked); + } - Self::log_audit( - &env, - &cert_id, - &record.issuer, - String::from_str(&env, "REVOKED"), - reason_code, - ); + let message = Self::revocation_message(&env, &cert_id, &reason_code); + // Traps on verification failure. + env.crypto() + .ed25519_verify(&institution_key, &message, &signature); + + Self::apply_revocation(&env, &mut record, reason_code); } pub fn reissue_certificate( env: Env, old_cert_id: BytesN<32>, new_content_hash: BytesN<32>, + new_metadata_uri: String, ) -> BytesN<32> { let mut old_record: CertificateRecord = env .storage() @@ -190,6 +260,8 @@ impl CertificateContract { revocation_reason: String::from_str(&env, ""), previous_cert_id: old_cert_id.clone(), content_hash: new_content_hash, + metadata_uri: new_metadata_uri, + expires_at: old_record.expires_at, royalty_bps: old_record.royalty_bps, soulbound: true, }; @@ -278,6 +350,48 @@ impl CertificateContract { .unwrap_or_else(|| panic_with_error!(&env, Error::NotFound)) } + /// Single-call verification status. Never panics for unknown IDs. + /// Precedence: `NotFound` > `Revoked` > `Expired` > `Valid`. + /// Certificates in `Reissued` status are treated as `Revoked` because + /// reissuance is only possible after revocation. + pub fn verify_certificate(env: Env, cert_id: BytesN<32>) -> VerificationStatus { + let record: CertificateRecord = match env + .storage() + .persistent() + .get(&DataKey::Certificate(cert_id)) + { + Some(r) => r, + None => return VerificationStatus::NotFound, + }; + + match record.status { + CertificateStatus::Revoked | CertificateStatus::Reissued => { + return VerificationStatus::Revoked; + } + CertificateStatus::Expired => return VerificationStatus::Expired, + CertificateStatus::Active => {} + } + + if let Some(expires_at) = record.expires_at { + if env.ledger().timestamp() >= expires_at { + return VerificationStatus::Expired; + } + } + + VerificationStatus::Valid + } + + /// Returns `true` if `content_hash` matches the hash recorded for the + /// certificate's metadata document. Traps if the certificate is unknown. + pub fn verify_content_hash(env: Env, cert_id: BytesN<32>, content_hash: BytesN<32>) -> bool { + let record: CertificateRecord = env + .storage() + .persistent() + .get(&DataKey::Certificate(cert_id)) + .unwrap_or_else(|| panic_with_error!(&env, Error::NotFound)); + record.content_hash == content_hash + } + pub fn get_merkle_root(env: Env, cohort_id: String) -> MerkleCohortRoot { env.storage() .persistent() @@ -331,18 +445,29 @@ impl CertificateContract { /// standard way to probe (and fail loudly on) transferability instead /// of relying on the absence of a `transfer` function. pub fn transfer_certificate(env: Env, cert_id: BytesN<32>, _to: Address) { - let record: CertificateRecord = env + let _record: CertificateRecord = env .storage() .persistent() .get(&DataKey::Certificate(cert_id)) .unwrap_or_else(|| panic_with_error!(&env, Error::NotFound)); - if record.soulbound { - panic_with_error!(&env, Error::SoulboundNonTransferable); - } - // Non-soulbound certificates are not supported by this contract - // today; reaching here would require a future opt-out mechanism. - panic_with_error!(&env, Error::SoulboundNonTransferable); + panic_with_error!(&env, Error::NonTransferable); + } + + /// Always panics with `NonTransferable`. Certificates cannot be moved. + pub fn transfer(env: Env, _from: Address, _to: Address, _cert_id: BytesN<32>) { + panic_with_error!(&env, Error::NonTransferable); + } + + /// Always panics with `NonTransferable`. Certificates cannot be moved. + pub fn transfer_from( + env: Env, + _spender: Address, + _from: Address, + _to: Address, + _cert_id: BytesN<32>, + ) { + panic_with_error!(&env, Error::NonTransferable); } /// Returns `true` if the certificate is soulbound (non-transferable). @@ -387,11 +512,54 @@ impl CertificateContract { } } +impl CertificateContract { + /// Marks the record revoked, persists it, appends an audit entry and + /// publishes a `CertificateRevokedEvent`. Callers must perform their + /// own authorization and status checks first. + fn apply_revocation(env: &Env, record: &mut CertificateRecord, reason_code: String) { + let cert_id = record.cert_id.clone(); + let issuer = record.issuer.clone(); + + record.status = CertificateStatus::Revoked; + record.revocation_reason = reason_code.clone(); + env.storage() + .persistent() + .set(&DataKey::Certificate(cert_id.clone()), &*record); + + Self::log_audit( + env, + &cert_id, + &issuer, + String::from_str(env, "REVOKED"), + reason_code.clone(), + ); + + env.events().publish( + (symbol_short!("cert_rev"),), + CertificateRevokedEvent { + cert_id, + issuer, + reason_code, + timestamp: env.ledger().timestamp(), + }, + ); + } + + /// Builds the signed message for institutional revocation: + /// `cert_id (32 bytes) || reason_code (UTF-8 bytes)`. + fn revocation_message(env: &Env, cert_id: &BytesN<32>, reason_code: &String) -> Bytes { + let mut message = Bytes::from_slice(env, &cert_id.to_array()); + message.append(&Bytes::from(reason_code.clone())); + message + } +} + #[contracttype] #[derive(Clone, Debug)] pub enum DataKey { Admin, NextCertId, + InstitutionKey, Certificate(BytesN<32>), MerkleRoot(String), AuditLog(BytesN<32>), diff --git a/contracts/certificate/src/test.rs b/contracts/certificate/src/test.rs index ae63cd2c0..a75f63ba3 100644 --- a/contracts/certificate/src/test.rs +++ b/contracts/certificate/src/test.rs @@ -1,7 +1,10 @@ #![cfg(test)] use super::*; -use soroban_sdk::{testutils::Address as _, Bytes, BytesN, Env, String, Vec}; +use soroban_sdk::{ + testutils::{Address as _, Ledger as _}, + Bytes, BytesN, Env, String, Vec, +}; /// Helper harness to set up the Soroban environment, register the contract, /// initialize the admin, and generate mock participants. @@ -62,8 +65,9 @@ fn test_issue_certificate_success() { let (env, client, _admin, issuer, owner) = setup_test(); let course_id = String::from_str(&env, "SOROBAN-101"); let content_hash = BytesN::from_array(&env, &[0xAA; 32]); + let uri = String::from_str(&env, "ipfs://bafy-cert-1"); - let cert_id = client.issue_certificate(&owner, &issuer, &course_id, &content_hash); + let cert_id = client.issue_certificate(&owner, &issuer, &course_id, &content_hash, &uri, &None); let record = client.get_certificate(&cert_id); assert_eq!(record.cert_id, cert_id); @@ -77,6 +81,8 @@ fn test_issue_certificate_success() { BytesN::from_array(&env, &[0u8; 32]) ); assert_eq!(record.content_hash, content_hash); + assert_eq!(record.metadata_uri, uri); + assert_eq!(record.expires_at, None); assert_eq!(record.royalty_bps, 0); assert!(record.soulbound); @@ -93,10 +99,11 @@ fn test_issue_multiple_certificates_duplicate_prevention() { let (env, client, _admin, issuer, owner) = setup_test(); let course_id = String::from_str(&env, "RUST-ADVANCED"); let content_hash = BytesN::from_array(&env, &[0xBB; 32]); + let uri = String::from_str(&env, "ipfs://cid"); - let cert_id_1 = client.issue_certificate(&owner, &issuer, &course_id, &content_hash); - let cert_id_2 = client.issue_certificate(&owner, &issuer, &course_id, &content_hash); - let cert_id_3 = client.issue_certificate(&owner, &issuer, &course_id, &content_hash); + let cert_id_1 = client.issue_certificate(&owner, &issuer, &course_id, &content_hash, &uri, &None); + let cert_id_2 = client.issue_certificate(&owner, &issuer, &course_id, &content_hash, &uri, &None); + let cert_id_3 = client.issue_certificate(&owner, &issuer, &course_id, &content_hash, &uri, &None); // Verify sequential unique IDs are generated for duplicate preventions assert_ne!(cert_id_1, cert_id_2); @@ -121,8 +128,9 @@ fn test_revoke_certificate_success() { let (env, client, _admin, issuer, owner) = setup_test(); let course_id = String::from_str(&env, "WEB3-BOOTCAMP"); let content_hash = BytesN::from_array(&env, &[0x11; 32]); + let uri = String::from_str(&env, "ipfs://cid"); - let cert_id = client.issue_certificate(&owner, &issuer, &course_id, &content_hash); + let cert_id = client.issue_certificate(&owner, &issuer, &course_id, &content_hash, &uri, &None); let reason = String::from_str(&env, "Academic dishonesty violation"); client.revoke_certificate(&cert_id, &reason); @@ -138,14 +146,33 @@ fn test_revoke_certificate_success() { assert_eq!(revoke_log.reason, reason); } +#[test] +fn test_revoke_emits_structured_event() { + let (env, client, _admin, issuer, owner) = setup_test(); + let cert_id = client.issue_certificate( + &owner, + &issuer, + &String::from_str(&env, "COURSE"), + &BytesN::from_array(&env, &[0x12; 32]), + &String::from_str(&env, "ipfs://cid"), + &None, + ); + let before = env.events().all().events().len(); + + client.revoke_certificate(&cert_id, &String::from_str(&env, "R1")); + + assert!(env.events().all().events().len() > before); +} + #[test] #[should_panic] fn test_revoke_already_revoked_fails() { let (env, client, _admin, issuer, owner) = setup_test(); let course_id = String::from_str(&env, "WEB3-BOOTCAMP"); let content_hash = BytesN::from_array(&env, &[0x11; 32]); + let uri = String::from_str(&env, "ipfs://cid"); - let cert_id = client.issue_certificate(&owner, &issuer, &course_id, &content_hash); + let cert_id = client.issue_certificate(&owner, &issuer, &course_id, &content_hash, &uri, &None); let reason = String::from_str(&env, "First revocation"); client.revoke_certificate(&cert_id, &reason); @@ -162,6 +189,52 @@ fn test_revoke_nonexistent_certificate_fails() { client.revoke_certificate(&fake_id, &reason); } +#[test] +fn test_revoke_with_institution_sig_without_key_fails() { + let (env, client, _admin, issuer, owner) = setup_test(); + let cert_id = client.issue_certificate( + &owner, + &issuer, + &String::from_str(&env, "COURSE"), + &BytesN::from_array(&env, &[0x13; 32]), + &String::from_str(&env, "ipfs://cid"), + &None, + ); + let sig = BytesN::from_array(&env, &[0u8; 64]); + + // No institution key registered: must be rejected. + let result = client.try_revoke_certificate_with_institution_sig( + &cert_id, + &String::from_str(&env, "R"), + &sig, + ); + assert!(result.is_err()); + assert_eq!(client.get_certificate(&cert_id).status, CertificateStatus::Active); +} + +#[test] +fn test_revoke_with_invalid_institution_sig_fails() { + let (env, client, _admin, issuer, owner) = setup_test(); + let cert_id = client.issue_certificate( + &owner, + &issuer, + &String::from_str(&env, "COURSE"), + &BytesN::from_array(&env, &[0x14; 32]), + &String::from_str(&env, "ipfs://cid"), + &None, + ); + client.set_institution_key(&BytesN::from_array(&env, &[0x21; 32])); + let sig = BytesN::from_array(&env, &[0u8; 64]); + + let result = client.try_revoke_certificate_with_institution_sig( + &cert_id, + &String::from_str(&env, "R"), + &sig, + ); + assert!(result.is_err()); + assert_eq!(client.get_certificate(&cert_id).status, CertificateStatus::Active); +} + // ============================================================================ // 3. REISSUE WORKFLOW TESTS // ============================================================================ @@ -172,12 +245,14 @@ fn test_reissue_certificate_success() { let course_id = String::from_str(&env, "MATH-201"); let old_hash = BytesN::from_array(&env, &[0x10; 32]); let new_hash = BytesN::from_array(&env, &[0x20; 32]); + let uri = String::from_str(&env, "ipfs://old"); + let new_uri = String::from_str(&env, "ipfs://new"); - let old_cert_id = client.issue_certificate(&owner, &issuer, &course_id, &old_hash); + let old_cert_id = client.issue_certificate(&owner, &issuer, &course_id, &old_hash, &uri, &None); client.set_royalty_bps(&old_cert_id, &300u32); // 3% client.revoke_certificate(&old_cert_id, &String::from_str(&env, "Name misspelled")); - let new_cert_id = client.reissue_certificate(&old_cert_id, &new_hash); + let new_cert_id = client.reissue_certificate(&old_cert_id, &new_hash, &new_uri); let old_record = client.get_certificate(&old_cert_id); assert_eq!(old_record.status, CertificateStatus::Reissued); @@ -188,6 +263,7 @@ fn test_reissue_certificate_success() { assert_eq!(new_record.issuer, issuer); assert_eq!(new_record.course_id, course_id); assert_eq!(new_record.content_hash, new_hash); + assert_eq!(new_record.metadata_uri, new_uri); assert_eq!(new_record.previous_cert_id, old_cert_id); assert_eq!(new_record.royalty_bps, 300); assert!(new_record.soulbound); @@ -206,9 +282,16 @@ fn test_reissue_active_certificate_fails() { let old_hash = BytesN::from_array(&env, &[0x10; 32]); let new_hash = BytesN::from_array(&env, &[0x20; 32]); - let cert_id = client.issue_certificate(&owner, &issuer, &course_id, &old_hash); + let cert_id = client.issue_certificate( + &owner, + &issuer, + &course_id, + &old_hash, + &String::from_str(&env, "ipfs://cid"), + &None, + ); // Reissuing an Active (non-revoked) cert must panic with Error::NotRevoked - client.reissue_certificate(&cert_id, &new_hash); + client.reissue_certificate(&cert_id, &new_hash, &String::from_str(&env, "ipfs://new")); } #[test] @@ -217,7 +300,7 @@ fn test_reissue_nonexistent_certificate_fails() { let (env, client, _admin, _issuer, _owner) = setup_test(); let fake_id = BytesN::from_array(&env, &[0x99; 32]); let new_hash = BytesN::from_array(&env, &[0x20; 32]); - client.reissue_certificate(&fake_id, &new_hash); + client.reissue_certificate(&fake_id, &new_hash, &String::from_str(&env, "ipfs://new")); } // ============================================================================ @@ -315,7 +398,14 @@ fn test_set_royalty_bps_and_calculate_royalty() { let (env, client, _admin, issuer, owner) = setup_test(); let course_id = String::from_str(&env, "ART-101"); let hash = BytesN::from_array(&env, &[0x55; 32]); - let cert_id = client.issue_certificate(&owner, &issuer, &course_id, &hash); + let cert_id = client.issue_certificate( + &owner, + &issuer, + &course_id, + &hash, + &String::from_str(&env, "ipfs://cid"), + &None, + ); // Initial royalty is 0 let (creator, seller) = client.calculate_royalty(&cert_id, &10_000i128); @@ -344,6 +434,8 @@ fn test_calculate_royalty_zero_and_negative_amounts() { &issuer, &String::from_str(&env, "COURSE"), &BytesN::from_array(&env, &[0x01; 32]), + &String::from_str(&env, "ipfs://cid"), + &None, ); client.set_royalty_bps(&cert_id, &500u32); @@ -365,6 +457,8 @@ fn test_set_royalty_bps_above_cap_fails() { &issuer, &String::from_str(&env, "COURSE"), &BytesN::from_array(&env, &[0x01; 32]), + &String::from_str(&env, "ipfs://cid"), + &None, ); // Setting royalty above MAX_ROYALTY_BPS (5000) should panic with Error::RoyaltyTooHigh @@ -399,14 +493,34 @@ fn test_is_soulbound_returns_true() { &issuer, &String::from_str(&env, "COURSE"), &BytesN::from_array(&env, &[0x01; 32]), + &String::from_str(&env, "ipfs://cid"), + &None, ); assert!(client.is_soulbound(&cert_id)); } #[test] -#[should_panic] -fn test_transfer_certificate_always_panics() { +fn test_transfer_certificate_panics_non_transferable() { + let (env, client, _admin, issuer, owner) = setup_test(); + let recipient = Address::generate(&env); + let cert_id = client.issue_certificate( + &owner, + &issuer, + &String::from_str(&env, "COURSE"), + &BytesN::from_array(&env, &[0x01; 32]), + &String::from_str(&env, "ipfs://cid"), + &None, + ); + + match client.try_transfer_certificate(&cert_id, &recipient) { + Err(Ok(e)) => assert_eq!(e, Error::NonTransferable), + _ => panic!("expected NonTransferable"), + } +} + +#[test] +fn test_transfer_and_transfer_from_panic_non_transferable() { let (env, client, _admin, issuer, owner) = setup_test(); let recipient = Address::generate(&env); let cert_id = client.issue_certificate( @@ -414,9 +528,18 @@ fn test_transfer_certificate_always_panics() { &issuer, &String::from_str(&env, "COURSE"), &BytesN::from_array(&env, &[0x01; 32]), + &String::from_str(&env, "ipfs://cid"), + &None, ); - client.transfer_certificate(&cert_id, &recipient); + match client.try_transfer(&owner, &recipient, &cert_id) { + Err(Ok(e)) => assert_eq!(e, Error::NonTransferable), + _ => panic!("expected NonTransferable from transfer"), + } + match client.try_transfer_from(&issuer, &owner, &recipient, &cert_id) { + Err(Ok(e)) => assert_eq!(e, Error::NonTransferable), + _ => panic!("expected NonTransferable from transfer_from"), + } } #[test] @@ -447,3 +570,103 @@ fn test_audit_log_nonexistent_returns_empty_vector() { let logs = client.get_audit_log(&fake_id); assert_eq!(logs.len(), 0); } + +// ============================================================================ +// 8. VERIFICATION, EXPIRATION & METADATA HASH TESTS +// ============================================================================ + +#[test] +fn test_verify_valid_without_expiry() { + let (env, client, _admin, issuer, owner) = setup_test(); + let cert_id = client.issue_certificate( + &owner, + &issuer, + &String::from_str(&env, "COURSE"), + &BytesN::from_array(&env, &[0x31; 32]), + &String::from_str(&env, "ipfs://cid"), + &None, + ); + + env.ledger().set_timestamp(1_000_000); + assert_eq!(client.verify_certificate(&cert_id), VerificationStatus::Valid); +} + +#[test] +fn test_verify_expires_at_boundary() { + let (env, client, _admin, issuer, owner) = setup_test(); + env.ledger().set_timestamp(100); + let cert_id = client.issue_certificate( + &owner, + &issuer, + &String::from_str(&env, "COURSE"), + &BytesN::from_array(&env, &[0x32; 32]), + &String::from_str(&env, "ipfs://cid"), + &Some(200u64), + ); + + env.ledger().set_timestamp(199); + assert_eq!(client.verify_certificate(&cert_id), VerificationStatus::Valid); + + // now >= expires_at means expired + env.ledger().set_timestamp(200); + assert_eq!(client.verify_certificate(&cert_id), VerificationStatus::Expired); + // Expiry does not alter the stored status + assert_eq!(client.get_certificate(&cert_id).status, CertificateStatus::Active); +} + +#[test] +fn test_verify_revoked_takes_precedence_over_expired() { + let (env, client, _admin, issuer, owner) = setup_test(); + env.ledger().set_timestamp(100); + let cert_id = client.issue_certificate( + &owner, + &issuer, + &String::from_str(&env, "COURSE"), + &BytesN::from_array(&env, &[0x33; 32]), + &String::from_str(&env, "ipfs://cid"), + &Some(150u64), + ); + client.revoke_certificate(&cert_id, &String::from_str(&env, "R")); + + env.ledger().set_timestamp(500); + assert_eq!(client.verify_certificate(&cert_id), VerificationStatus::Revoked); +} + +#[test] +fn test_verify_revoked_status() { + let (env, client, _admin, issuer, owner) = setup_test(); + let cert_id = client.issue_certificate( + &owner, + &issuer, + &String::from_str(&env, "COURSE"), + &BytesN::from_array(&env, &[0x34; 32]), + &String::from_str(&env, "ipfs://cid"), + &None, + ); + client.revoke_certificate(&cert_id, &String::from_str(&env, "R")); + assert_eq!(client.verify_certificate(&cert_id), VerificationStatus::Revoked); +} + +#[test] +fn test_verify_not_found_for_unknown_id() { + let (env, client, _admin, _issuer, _owner) = setup_test(); + let fake_id = BytesN::from_array(&env, &[0xDD; 32]); + assert_eq!(client.verify_certificate(&fake_id), VerificationStatus::NotFound); +} + +#[test] +fn test_verify_content_hash_match_and_mismatch() { + let (env, client, _admin, issuer, owner) = setup_test(); + let hash = BytesN::from_array(&env, &[0x41; 32]); + let cert_id = client.issue_certificate( + &owner, + &issuer, + &String::from_str(&env, "COURSE"), + &hash, + &String::from_str(&env, "ipfs://cid"), + &None, + ); + + assert!(client.verify_content_hash(&cert_id, &hash)); + assert!(!client.verify_content_hash(&cert_id, &BytesN::from_array(&env, &[0x42; 32]))); +} diff --git a/contracts/certificate_nft/src/badges.rs b/contracts/certificate_nft/src/badges.rs index 0701f31b0..b78cb62c9 100644 --- a/contracts/certificate_nft/src/badges.rs +++ b/contracts/certificate_nft/src/badges.rs @@ -116,6 +116,6 @@ impl AchievementBadge { /// Soulbound assertion: transferring this badge is strictly disallowed. pub fn assert_transfer_blocked(env: &Env) -> ! { - panic_with_error!(env, CertError::SoulboundNonTransferable); + panic_with_error!(env, CertError::NonTransferable); } } diff --git a/contracts/certificate_nft/src/lib.rs b/contracts/certificate_nft/src/lib.rs index 1271edf31..644ca00cd 100644 --- a/contracts/certificate_nft/src/lib.rs +++ b/contracts/certificate_nft/src/lib.rs @@ -10,7 +10,7 @@ use soroban_sdk::{ pub use badges::{AchievementBadge, ProvenanceRecord}; pub use rarity::RarityTier; -// ── Errors (SC-HARD-20: range 400+) ────────────────────────────────────────── +// ---- Errors (SC-HARD-20: range 400+) ---- /// Typed contract errors for the certificate NFT contract. /// @@ -19,18 +19,20 @@ pub use rarity::RarityTier; #[derive(Copy, Clone, Debug, Eq, PartialEq)] #[repr(u32)] pub enum CertError { - /// `400` — A certificate has already been minted for this student and course. + /// `400` - A certificate has already been minted for this student and course. AlreadyMinted = 400, - /// `401` — Caller is not the admin. + /// `401` - Caller is not the admin. Unauthorized = 401, - /// `402` — Badge is soulbound and cannot be transferred. + /// `402` - Badge is soulbound and cannot be transferred. SoulboundNonTransferable = 402, - /// `403` — Requested tier upgrade is invalid or prerequisites unmet. + /// `403` - Requested tier upgrade is invalid or prerequisites unmet. InvalidTierUpgrade = 403, - /// `404` — Requested badge ID was not found. + /// `404` - Requested badge ID was not found. BadgeNotFound = 404, - /// `405` — Insufficient completed milestones for this tier. + /// `405` - Insufficient completed milestones for this tier. InsufficientMilestones = 405, + /// `406` - Badge transfer attempted; badges are non-transferable. + NonTransferable = 406, } #[contracttype] @@ -114,50 +116,7 @@ impl CertificateNFTContract { (token_id, current_recipient, new_recipient), ); } -} - -#[cfg(test)] -mod tests { - use super::*; - use soroban_sdk::testutils::Address as _; - - #[test] - fn test_e2e_mint_and_royalty_calculation() { - let env = Env::default(); - env.mock_all_auths(); - let contract_id = env.register(CertificateNFTContract, ()); - let client = CertificateNFTContractClient::new(&env, &contract_id); - - let admin = Address::generate(&env); - let student = Address::generate(&env); - let designer = Address::generate(&env); - let course_id = String::from_str(&env, "CS101"); - let token_id = String::from_str(&env, "badge-cs101"); - - client.mint_certificate( - &admin, - &student, - &course_id, - &String::from_str(&env, "A"), - &String::from_str(&env, "ipfs://badge101"), - ); - - // Configure 5% (500 bps) royalty to designer - client.set_token_royalty(&admin, &token_id, &designer, &500u32); - - // Check royalty on secondary sale of 10,000 stroops/units - // 5% of 10_000 = 500 - let royalty = client.royalty_info(&token_id, &10_000u128); - assert_eq!(royalty.recipient, Some(designer.clone())); - assert_eq!(royalty.royalty_amount, 500u128); - // Test update payout address - let new_designer = Address::generate(&env); - client.update_royalty_recipient(&token_id, &designer, &new_designer); - - let updated_royalty = client.royalty_info(&token_id, &20_000u128); - assert_eq!(updated_royalty.recipient, Some(new_designer)); - assert_eq!(updated_royalty.royalty_amount, 1_000u128); /// Mints a new tiered achievement badge to a student. pub fn mint_badge( env: Env, @@ -238,5 +197,50 @@ mod tests { } } +#[cfg(test)] +mod tests { + use super::*; + use soroban_sdk::testutils::Address as _; + + #[test] + fn test_e2e_mint_and_royalty_calculation() { + let env = Env::default(); + env.mock_all_auths(); + let contract_id = env.register(CertificateNFTContract, ()); + let client = CertificateNFTContractClient::new(&env, &contract_id); + + let admin = Address::generate(&env); + let student = Address::generate(&env); + let designer = Address::generate(&env); + let course_id = String::from_str(&env, "CS101"); + let token_id = String::from_str(&env, "badge-cs101"); + + client.mint_certificate( + &admin, + &student, + &course_id, + &String::from_str(&env, "A"), + &String::from_str(&env, "ipfs://badge101"), + ); + + // Configure 5% (500 bps) royalty to designer + client.set_token_royalty(&admin, &token_id, &designer, &500u32); + + // Check royalty on secondary sale of 10,000 stroops/units + // 5% of 10_000 = 500 + let royalty = client.royalty_info(&token_id, &10_000u128); + assert_eq!(royalty.recipient, Some(designer.clone())); + assert_eq!(royalty.royalty_amount, 500u128); + + // Test update payout address + let new_designer = Address::generate(&env); + client.update_royalty_recipient(&token_id, &designer, &new_designer); + + let updated_royalty = client.royalty_info(&token_id, &20_000u128); + assert_eq!(updated_royalty.recipient, Some(new_designer)); + assert_eq!(updated_royalty.royalty_amount, 1_000u128); + } +} + #[cfg(test)] mod test; diff --git a/contracts/certificate_nft/src/test.rs b/contracts/certificate_nft/src/test.rs index 87b47a05c..6b2e7dec4 100644 --- a/contracts/certificate_nft/src/test.rs +++ b/contracts/certificate_nft/src/test.rs @@ -39,7 +39,7 @@ fn test_mint_badge_and_soulbound_enforcement() { } #[test] -#[should_panic(expected = "Error(Contract, #402)")] +#[should_panic(expected = "Error(Contract, #406)")] fn test_badge_transfer_fails_soulbound() { let (env, client, admin, student) = setup(); let recipient = Address::generate(&env); @@ -57,10 +57,28 @@ fn test_badge_transfer_fails_soulbound() { &milestone1, ); - // Transfer must panic with SoulboundNonTransferable (402) + // Transfer must panic with NonTransferable (406) client.transfer(&student, &recipient, &badge_id); } +#[test] +#[should_panic(expected = "Error(Contract, #406)")] +fn test_badge_transfer_from_fails_non_transferable() { + let (env, client, admin, student) = setup(); + let recipient = Address::generate(&env); + + let badge_id = client.mint_badge( + &admin, + &student, + &String::from_str(&env, "SORO-101"), + &RarityTier::Bronze, + &String::from_str(&env, "ipfs://cid1"), + &String::from_str(&env, "Milestone 1"), + ); + + client.transfer_from(&admin, &student, &recipient, &badge_id); +} + #[test] fn test_badge_tier_upgrade_and_provenance_history() { let (env, client, admin, student) = setup(); From 1b7ec243caef895fe2d906958b0b1c0d8a0dbe3f Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Sat, 17 Oct 2026 16:29:05 +0100 Subject: [PATCH 3/4] feat(dao): proposal threshold, quorum, 48h timelock, and vote snapshot (#1625) - Minimum proposal threshold and bounded voting period (default 7 days) - Quorum of 20% of granted credits; 48h timelock before execute - Grants after proposal creation do not add voting power - Resolve duplicate DaoError and fix syntax error in lib.rs --- contracts/dao_governance/src/errors.rs | 40 +- contracts/dao_governance/src/lib.rs | 502 ++++++++++++------------- contracts/dao_governance/src/test.rs | 481 ++++++++++++++++------- 3 files changed, 608 insertions(+), 415 deletions(-) diff --git a/contracts/dao_governance/src/errors.rs b/contracts/dao_governance/src/errors.rs index 2f652b066..1ed182c28 100644 --- a/contracts/dao_governance/src/errors.rs +++ b/contracts/dao_governance/src/errors.rs @@ -1,8 +1,8 @@ //! # DAO Governance Error Definitions //! //! Standardized, typed, and documented contract errors for DAO governance, -//! voting, proposals, and voice credit operations. -//! Namespace: 400–499 (Governance). +//! voting, proposals, timelock, and voice credit operations. +//! Namespace: 400-499 (Governance). This is the single `DaoError` for the crate. use soroban_sdk::contracterror; @@ -11,24 +11,40 @@ use soroban_sdk::contracterror; #[derive(Copy, Clone, Debug, Eq, PartialEq)] #[repr(u32)] pub enum DaoError { - /// 401 — Contract has already been initialized. + /// 401 - Contract has already been initialized. AlreadyInitialized = 401, - /// 402 — Contract has not been initialized. + /// 402 - Contract has not been initialized. NotInitialized = 402, - /// 403 — Caller is not authorized for governance operation. + /// 403 - Caller is not authorized for governance operation. Unauthorized = 403, - /// 404 — Proposal was not found with specified identifier. + /// 404 - Proposal was not found with specified identifier. ProposalNotFound = 404, - /// 405 — Proposal is closed and not open for voting or execution. + /// 405 - Proposal is not in the status required by the operation. ProposalClosed = 405, - /// 406 — Voting deadline is still live; premature finalization rejected. + /// 406 - Voting deadline is still live; premature finalization rejected. VotingDeadlineLive = 406, - /// 407 — Voter has already cast votes on this proposal. + /// 407 - Voter has already cast votes on this proposal (or replayed a nonce). AlreadyVoted = 407, - /// 408 — Insufficient voice credits available to cast the requested votes. + /// 408 - Insufficient spendable voice credits for the requested votes. InsufficientCredits = 408, - /// 409 — Number of votes must be strictly positive. + /// 409 - Number of votes must be non-zero. ZeroVotes = 409, - /// 410 — Proposal deadline is invalid or not in the future. + /// 410 - Invalid deadline (reserved; zero duration now selects the default period). InvalidDeadline = 410, + /// 411 - Ragequit requested with zero shares. + ZeroShares = 411, + /// 412 - Proposal is not in Passed (queued) status. + ProposalNotPassed = 412, + /// 413 - Voter cast a positive (Yes) vote and may not ragequit. + YesVoterCannotRagequit = 413, + /// 414 - Voter does not hold enough treasury shares. + InsufficientShares = 414, + /// 415 - Treasury has no reserves to pay out. + InsufficientReserves = 415, + /// 416 - Creator holds fewer credits than the proposal threshold. + BelowProposalThreshold = 416, + /// 417 - Voting period is outside the allowed bounds (1 to 30 days). + InvalidVotingPeriod = 417, + /// 418 - Timelock delay (48 hours) has not elapsed since the proposal was queued. + TimelockNotElapsed = 418, } diff --git a/contracts/dao_governance/src/lib.rs b/contracts/dao_governance/src/lib.rs index c8dfd56f6..475bd60d1 100644 --- a/contracts/dao_governance/src/lib.rs +++ b/contracts/dao_governance/src/lib.rs @@ -1,32 +1,68 @@ -//! # DAO Governance Module with Quadratic Voting +//! # DAO Governance Module with Quadratic Voting, Quorum and Timelock //! -//! Closes issue #717. +//! Closes issues #717 and #1625. //! -//! A Soroban-native DAO where any token holder can: -//! - create proposals with a configurable voting window, -//! - cast votes whose *cost* scales quadratically (cost = votes²), -//! so large stakeholders cannot dominate by raw token weight, -//! - finalize proposals once the deadline passes, -//! - execute passed proposals (hook for downstream actions). +//! A Soroban-native DAO where any voice-credit holder can: +//! - create proposals (creator must hold at least the proposal threshold), +//! - cast votes whose *cost* scales quadratically (cost = votes^2), +//! - finalize proposals once the voting period ends, +//! - execute passed proposals once the 48-hour timelock has elapsed. +//! +//! ## Parameters +//! - Default voting period: 7 days (`DEFAULT_VOTING_PERIOD`). Custom periods +//! (set by admin, or passed as `duration` to `create_proposal`) must lie in +//! `[MIN_VOTING_PERIOD, MAX_VOTING_PERIOD]` = [1 day, 30 days]. +//! - Quorum: a proposal passes only if the voice credits spent on it +//! (`credits_spent`, i.e. participating voting power) reach +//! `QUORUM_BPS` (20%) of the circulating governance supply, where supply is +//! the running total of all credits ever granted (`TotalGranted`). +//! The quorum is fixed at proposal creation. +//! - Timelock: finalize queues a passed proposal with `eta = now + 172800` +//! (48 hours). `execute` fails until `now >= eta`. +//! +//! ## Flash-loan protection (creation-time snapshot) +//! Every grant is stamped with a monotonically increasing sequence number and +//! appended to the member's grant log. Each proposal records the sequence +//! number at creation (`created_seq`). When a member votes, credits granted +//! with a sequence number greater than `created_seq` are NOT spendable on that +//! proposal: spendable = current balance - (sum of grants after creation). +//! Voting power therefore cannot increase because of grants made after the +//! proposal was created. This is conservative: if a post-creation grant was +//! already spent on another proposal, it is still subtracted here. //! //! ## Quadratic voting mechanics -//! Each voter is allocated `voice_credits` upon joining. Casting `v` votes -//! on a proposal costs `v²` credits, forcing voters to spread influence -//! across multiple proposals rather than concentrating on one. +//! Casting `v` votes on a proposal costs `v^2` credits, forcing voters to spread +//! influence across multiple proposals rather than concentrating on one. #![no_std] use soroban_sdk::{ - contract, contractimpl, contracttype, panic_with_error, symbol_short, Address, - Env, String, + contract, contractimpl, contracttype, panic_with_error, symbol_short, Address, Env, String, + Vec, }; +// ── Constants ───────────────────────────────────────────────────────────────── + +/// Default voting period: 7 days. +pub const DEFAULT_VOTING_PERIOD: u64 = 604_800; +/// Minimum allowed voting period: 1 day. +pub const MIN_VOTING_PERIOD: u64 = 86_400; +/// Maximum allowed voting period: 30 days. +pub const MAX_VOTING_PERIOD: u64 = 2_592_000; +/// Mandatory timelock between finalize (queue) and execute: 48 hours. +pub const TIMELOCK_DELAY: u64 = 172_800; +/// Quorum as basis points of circulating supply: 20%. +pub const QUORUM_BPS: u128 = 2_000; +/// Basis-point denominator. +pub const BPS_DENOMINATOR: u128 = 10_000; + // ── Types ───────────────────────────────────────────────────────────────────── #[contracttype] #[derive(Clone, Debug, Eq, PartialEq)] pub enum ProposalStatus { Active, + /// Passed and queued in the timelock (`eta` set); executable once `now >= eta`. Passed, Failed, Executed, @@ -43,8 +79,23 @@ pub struct Proposal { pub status: ProposalStatus, /// Net vote tally (positive = support, negative = against). pub tally: i128, - /// Total voice-credits spent across all voters on this proposal. + /// Total voice-credits spent across all voters on this proposal + /// (participating voting power used for quorum). pub credits_spent: u128, + /// Grant sequence number at creation. Later grants are not spendable here. + pub created_seq: u64, + /// Quorum in voice credits, fixed at creation (QUORUM_BPS of supply). + pub quorum_required: u128, + /// Timelock ETA (ledger timestamp) set when passed; 0 while not queued. + pub eta: u64, +} + +/// One credit grant, stamped with its global sequence number. +#[contracttype] +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct GrantEntry { + pub seq: u64, + pub amount: u128, } #[contracttype] @@ -56,38 +107,26 @@ pub enum Key { Proposal(u64), Vote(u64, Address), UsedNonce(Address, u64), + /// Running total of all credits granted (circulating supply). + TotalGranted, + /// Global grant/proposal sequence counter. + Seq, + /// Per-member list of grants with sequence numbers. + GrantLog(Address), + /// Admin-configured default voting period (seconds). + VotingPeriod, + /// Minimum credits a proposal creator must hold. + ProposalThreshold, } +pub mod errors; pub mod ragequit; pub mod treasury; +pub use errors::DaoError; use ragequit::Ragequit; use treasury::{Treasury, TreasuryState}; -// ── Errors ──────────────────────────────────────────────────────────────────── - -#[contracterror] -#[derive(Copy, Clone, Debug, Eq, PartialEq)] -pub enum DaoError { - AlreadyInitialized = 1, - NotInitialized = 2, - Unauthorized = 3, - ProposalNotFound = 4, - ProposalClosed = 5, - VotingDeadlineLive = 6, - AlreadyVoted = 7, - InsufficientCredits = 8, - ZeroVotes = 9, - InvalidDeadline = 10, - ZeroShares = 11, - ProposalNotPassed = 12, - YesVoterCannotRagequit = 13, - InsufficientShares = 14, - InsufficientReserves = 15, -} -pub mod errors; -pub use errors::DaoError; - // ── Contract ────────────────────────────────────────────────────────────────── #[contract] @@ -106,18 +145,41 @@ impl DaoGovernance { } /// Grant `credits` voice-credits to `member`. Admin only. + /// Updates the circulating supply and the member's grant log. pub fn grant_credits(env: Env, member: Address, credits: u128) { Self::only_admin(&env); let key = Key::Credits(member.clone()); let prev: u128 = env.storage().persistent().get(&key).unwrap_or(0); - env.storage().persistent().set(&key, &(prev + credits)); + env.storage() + .persistent() + .set(&key, &prev.saturating_add(credits)); + + let total = read_total_granted(&env).saturating_add(credits); + env.storage().instance().set(&Key::TotalGranted, &total); + + let seq = next_seq(&env); + let log_key = Key::GrantLog(member.clone()); + let mut log = env + .storage() + .persistent() + .get::>(&log_key) + .unwrap_or_else(|| Vec::new(&env)); + log.push_back(GrantEntry { + seq, + amount: credits, + }); + env.storage().persistent().set(&log_key, &log); + env.events() .publish((symbol_short!("credits"),), (member, credits)); } /// Create a new proposal. Returns the proposal ID. /// - /// * `duration` – voting window in seconds. + /// * `duration` - voting window in seconds. `0` selects the configured + /// default voting period (7 days unless changed by admin). Any non-zero + /// value must be within [1 day, 30 days], otherwise `InvalidVotingPeriod`. + /// * The creator must hold at least the proposal threshold (default 0). pub fn create_proposal( env: Env, creator: Address, @@ -126,21 +188,36 @@ impl DaoGovernance { duration: u64, ) -> u64 { creator.require_auth(); - if duration == 0 { - panic_with_error!(&env, DaoError::InvalidDeadline); + + let period = if duration == 0 { + read_voting_period(&env) + } else { + check_voting_period(&env, duration); + duration + }; + + if credits_balance(&env, &creator) < read_proposal_threshold(&env) { + panic_with_error!(&env, DaoError::BelowProposalThreshold); } + let id: u64 = env.storage().instance().get(&Key::NextId).unwrap_or(0); env.storage().instance().set(&Key::NextId, &(id + 1)); + let quorum_required = + read_total_granted(&env).saturating_mul(QUORUM_BPS) / BPS_DENOMINATOR; + let proposal = Proposal { id, creator: creator.clone(), title, description, - deadline: env.ledger().timestamp() + duration, + deadline: env.ledger().timestamp().saturating_add(period), status: ProposalStatus::Active, tally: 0, credits_spent: 0, + created_seq: next_seq(&env), + quorum_required, + eta: 0, }; env.storage() .persistent() @@ -153,13 +230,9 @@ impl DaoGovernance { /// Cast `votes` on proposal `id`. /// /// * Positive = support, negative = against. - /// * Cost = votes² credits (quadratic scaling). - /// * Each address may vote at most once per proposal. - /// Cast `votes` on proposal `id`. - /// - /// * Positive = support, negative = against. - /// * Cost = votes² credits (quadratic scaling). + /// * Cost = votes^2 credits (quadratic scaling). /// * Each address may vote at most once per proposal. + /// * Only credits not granted after the proposal was created are spendable. pub fn vote(env: Env, voter: Address, proposal_id: u64, votes: i64) { voter.require_auth(); Self::internal_vote(&env, voter, proposal_id, votes); @@ -204,16 +277,19 @@ impl DaoGovernance { panic_with_error!(env, DaoError::AlreadyVoted); } - // Quadratic cost: cost = |votes|² + // Quadratic cost: cost = |votes|^2 let abs_v = votes.unsigned_abs() as u128; let cost: u128 = abs_v.saturating_mul(abs_v); - let credits_key = Key::Credits(voter.clone()); - let bal: u128 = env.storage().persistent().get(&credits_key).unwrap_or(0); - if bal < cost { + let spendable = eligible_credits(env, &voter, proposal.created_seq); + if spendable < cost { panic_with_error!(env, DaoError::InsufficientCredits); } - env.storage().persistent().set(&credits_key, &(bal - cost)); + let credits_key = Key::Credits(voter.clone()); + let bal = credits_balance(env, &voter); + env.storage() + .persistent() + .set(&credits_key, &bal.saturating_sub(cost)); proposal.tally += votes as i128; proposal.credits_spent = proposal.credits_spent.saturating_add(cost); @@ -227,6 +303,11 @@ impl DaoGovernance { } /// Finalize a proposal after its deadline. Anyone may call this. + /// + /// The proposal passes only if `credits_spent >= quorum_required` AND + /// `tally > 0`. A passing proposal is queued: status `Passed` with + /// `eta = now + TIMELOCK_DELAY`. Otherwise it is marked `Failed` + /// (including quorum failure); no panic is raised for that outcome. pub fn finalize(env: Env, proposal_id: u64) { let mut proposal: Proposal = env .storage() @@ -237,15 +318,17 @@ impl DaoGovernance { if proposal.status != ProposalStatus::Active { panic_with_error!(&env, DaoError::ProposalClosed); } - if env.ledger().timestamp() <= proposal.deadline { + let now = env.ledger().timestamp(); + if now <= proposal.deadline { panic_with_error!(&env, DaoError::VotingDeadlineLive); } - proposal.status = if proposal.tally > 0 { - ProposalStatus::Passed + if proposal.credits_spent >= proposal.quorum_required && proposal.tally > 0 { + proposal.status = ProposalStatus::Passed; + proposal.eta = now.saturating_add(TIMELOCK_DELAY); } else { - ProposalStatus::Failed - }; + proposal.status = ProposalStatus::Failed; + } env.storage() .persistent() .set(&Key::Proposal(proposal_id), &proposal); @@ -253,7 +336,10 @@ impl DaoGovernance { .publish((symbol_short!("finalize"),), (proposal_id, proposal.tally)); } - /// Mark a passed proposal as executed. Admin only. + /// Mark a queued (passed) proposal as executed. Admin only. + /// + /// Fails with `ProposalClosed` unless the status is `Passed`, and with + /// `TimelockNotElapsed` until `now >= eta` (48 hours after finalize). pub fn execute(env: Env, caller: Address, proposal_id: u64) { Self::only_admin(&env); caller.require_auth(); @@ -267,6 +353,9 @@ impl DaoGovernance { if proposal.status != ProposalStatus::Passed { panic_with_error!(&env, DaoError::ProposalClosed); } + if env.ledger().timestamp() < proposal.eta { + panic_with_error!(&env, DaoError::TimelockNotElapsed); + } proposal.status = ProposalStatus::Executed; env.storage() .persistent() @@ -275,6 +364,27 @@ impl DaoGovernance { .publish((symbol_short!("execute"),), (caller, proposal_id)); } + // ── Configuration (admin) ───────────────────────────────────────────────── + + /// Set the default voting period. Must be within [1 day, 30 days]. Admin only. + pub fn set_voting_period(env: Env, period: u64) { + Self::only_admin(&env); + check_voting_period(&env, period); + env.storage().instance().set(&Key::VotingPeriod, &period); + env.events() + .publish((symbol_short!("vp_set"),), period); + } + + /// Set the minimum credits a proposal creator must hold. Admin only. + pub fn set_proposal_threshold(env: Env, min_credits: u128) { + Self::only_admin(&env); + env.storage() + .instance() + .set(&Key::ProposalThreshold, &min_credits); + env.events() + .publish((symbol_short!("thr_set"),), min_credits); + } + // ── Views ───────────────────────────────────────────────────────────────── pub fn get_proposal(env: Env, id: u64) -> Option { @@ -282,10 +392,7 @@ impl DaoGovernance { } pub fn credits_of(env: Env, member: Address) -> u128 { - env.storage() - .persistent() - .get(&Key::Credits(member)) - .unwrap_or(0) + credits_balance(&env, &member) } pub fn vote_of(env: Env, proposal_id: u64, voter: Address) -> Option { @@ -294,6 +401,19 @@ impl DaoGovernance { .get(&Key::Vote(proposal_id, voter)) } + /// Circulating governance supply: total credits ever granted. + pub fn total_granted(env: Env) -> u128 { + read_total_granted(&env) + } + + pub fn get_voting_period(env: Env) -> u64 { + read_voting_period(&env) + } + + pub fn get_proposal_threshold(env: Env) -> u128 { + read_proposal_threshold(&env) + } + // ── Treasury & Ragequit (SC-042) ────────────────────────────────────────── pub fn deposit_treasury(env: Env, caller: Address, amount: u128) { @@ -340,218 +460,68 @@ impl DaoGovernance { } } -// ── Tests ───────────────────────────────────────────────────────────────────── - -#[cfg(test)] -mod tests { - use super::*; - use soroban_sdk::{ - testutils::{Address as _, Ledger as _}, - Env, String, - }; - - fn setup() -> (Env, DaoGovernanceClient<'static>, Address) { - let env = Env::default(); - env.mock_all_auths(); - let id = env.register(DaoGovernance, ()); - let client = DaoGovernanceClient::new(&env, &id); - let admin = Address::generate(&env); - client.initialize(&admin); - (env, client, admin) - } - - #[test] - fn quadratic_cost_deducted_correctly() { - let (env, client, admin) = setup(); - let voter = Address::generate(&env); - client.grant_credits(&voter, &100u128); - - let pid = client.create_proposal( - &admin, - &String::from_str(&env, "Test"), - &String::from_str(&env, "Desc"), - &3600, - ); - - // 3 votes → cost = 9 - client.vote(&voter, &pid, &3i64); - assert_eq!(client.credits_of(&voter), 91); - - let p = client.get_proposal(&pid).unwrap(); - assert_eq!(p.tally, 3); - assert_eq!(p.credits_spent, 9); - } - - #[test] - fn proposal_passes_when_tally_positive() { - let (env, client, admin) = setup(); - let voter = Address::generate(&env); - client.grant_credits(&voter, &1_000u128); - - let pid = client.create_proposal( - &admin, - &String::from_str(&env, "Upgrade"), - &String::from_str(&env, "Details"), - &100, - ); - client.vote(&voter, &pid, &5i64); - - env.ledger().with_mut(|l| l.timestamp += 200); - client.finalize(&pid); - - assert_eq!( - client.get_proposal(&pid).unwrap().status, - ProposalStatus::Passed - ); - } - - #[test] - fn proposal_fails_when_tally_non_positive() { - let (env, client, admin) = setup(); - let voter = Address::generate(&env); - client.grant_credits(&voter, &1_000u128); - - let pid = client.create_proposal( - &admin, - &String::from_str(&env, "Bad idea"), - &String::from_str(&env, "No"), - &100, - ); - client.vote(&voter, &pid, &-4i64); - - env.ledger().with_mut(|l| l.timestamp += 200); - client.finalize(&pid); +// ── Helpers ─────────────────────────────────────────────────────────────────── - assert_eq!( - client.get_proposal(&pid).unwrap().status, - ProposalStatus::Failed - ); - } +/// Advance and return the global grant/proposal sequence counter. +fn next_seq(env: &Env) -> u64 { + let seq: u64 = env.storage().instance().get(&Key::Seq).unwrap_or(0) + 1; + env.storage().instance().set(&Key::Seq, &seq); + seq +} - #[test] - #[should_panic(expected = "Error(Contract, #407)")] - fn cannot_vote_twice() { - let (env, client, admin) = setup(); - let voter = Address::generate(&env); - client.grant_credits(&voter, &1_000u128); - - let pid = client.create_proposal( - &admin, - &String::from_str(&env, "D"), - &String::from_str(&env, "D"), - &3600, - ); - client.vote(&voter, &pid, &1i64); - client.vote(&voter, &pid, &1i64); // panic - } +fn read_total_granted(env: &Env) -> u128 { + env.storage() + .instance() + .get(&Key::TotalGranted) + .unwrap_or(0) +} - #[test] - #[should_panic(expected = "Error(Contract, #408)")] - fn insufficient_credits_rejected() { - let (env, client, admin) = setup(); - let voter = Address::generate(&env); - client.grant_credits(&voter, &3u128); // only 3 credits - - let pid = client.create_proposal( - &admin, - &String::from_str(&env, "Big"), - &String::from_str(&env, "Big"), - &3600, - ); - client.vote(&voter, &pid, &5i64); // cost = 25 > 3 → panic - } +fn read_voting_period(env: &Env) -> u64 { + env.storage() + .instance() + .get(&Key::VotingPeriod) + .unwrap_or(DEFAULT_VOTING_PERIOD) +} - #[test] - fn test_ragequit_dissenting_voter_burns_shares_and_receives_pro_rata() { - let (env, client, admin) = setup(); - let voter_no = Address::generate(&env); - let voter_yes = Address::generate(&env); - - client.grant_credits(&voter_no, &1_000u128); - client.grant_credits(&voter_yes, &1_000u128); - - // Setup treasury: 1000 shares total (400 for voter_no, 600 for voter_yes), 50,000 reserves - client.grant_shares(&voter_no, &400u128); - client.grant_shares(&voter_yes, &600u128); - client.deposit_treasury(&admin, &50_000u128); - - let pid = client.create_proposal( - &admin, - &String::from_str(&env, "Controversial proposal"), - &String::from_str(&env, "Details"), - &100, - ); +fn read_proposal_threshold(env: &Env) -> u128 { + env.storage() + .instance() + .get(&Key::ProposalThreshold) + .unwrap_or(0) +} - // voter_no votes -2 (against), voter_yes votes +5 (for) - client.vote(&voter_no, &pid, &-2i64); - client.vote(&voter_yes, &pid, &5i64); - - // Proposal passes after deadline - env.ledger().with_mut(|l| l.timestamp += 200); - client.finalize(&pid); - assert_eq!(client.get_proposal(&pid).unwrap().status, ProposalStatus::Passed); - - // voter_no ragequits before proposal is executed: burns 400 shares - // Expected payout: 400 * 50,000 / 1000 = 20,000 - let payout = client.ragequit(&voter_no, &pid, &400u128); - assert_eq!(payout, 20_000u128); - - // Shares burned atomically - assert_eq!(client.treasury_shares_of(&voter_no), 0); - let treasury = client.get_treasury_state(); - assert_eq!(treasury.total_shares, 600); - assert_eq!(treasury.total_reserves, 30_000); +fn check_voting_period(env: &Env, period: u64) { + if period < MIN_VOTING_PERIOD || period > MAX_VOTING_PERIOD { + panic_with_error!(env, DaoError::InvalidVotingPeriod); } +} - #[test] - #[should_panic(expected = "Error(Contract, #13)")] - fn test_ragequit_prohibits_yes_voters() { - let (env, client, admin) = setup(); - let voter_yes = Address::generate(&env); - client.grant_credits(&voter_yes, &1_000u128); - client.grant_shares(&voter_yes, &500u128); - client.deposit_treasury(&admin, &50_000u128); - - let pid = client.create_proposal( - &admin, - &String::from_str(&env, "P"), - &String::from_str(&env, "D"), - &100, - ); - client.vote(&voter_yes, &pid, &5i64); - - env.ledger().with_mut(|l| l.timestamp += 200); - client.finalize(&pid); - - // Should panic with YesVoterCannotRagequit (code 13) - client.ragequit(&voter_yes, &pid, &100u128); - fn test_vote_sep0010_and_replay_rejection() { - let (env, client, admin) = setup(); - let voter = Address::generate(&env); - client.grant_credits(&voter, &100u128); - - let pid = client.create_proposal( - &admin, - &String::from_str(&env, "SEP10 Prop"), - &String::from_str(&env, "Desc"), - &3600, - ); - - let nonce = 9999u64; - let domain = String::from_str(&env, "governance.studentlab.stellar.org"); - - // Cast SEP-0010 authenticated vote - client.vote_sep0010(&voter, &pid, &3i64, &nonce, &domain); - - let prop = client.get_proposal(&pid).unwrap(); - assert_eq!(prop.tally, 3); - assert_eq!(client.credits_of(&voter), 100 - 9); +fn credits_balance(env: &Env, member: &Address) -> u128 { + env.storage() + .persistent() + .get::(&Key::Credits(member.clone())) + .unwrap_or(0) +} - // Replay attempt with same nonce is rejected - let res = client.try_vote_sep0010(&voter, &pid, &1i64, &nonce, &domain); - assert!(res.is_err()); +/// Credits `voter` may spend on a proposal created at `created_seq`: +/// current balance minus every grant received after that proposal was created. +fn eligible_credits(env: &Env, voter: &Address, created_seq: u64) -> u128 { + let balance = credits_balance(env, voter); + let log = env + .storage() + .persistent() + .get::>(&Key::GrantLog(voter.clone())) + .unwrap_or_else(|| Vec::new(env)); + let mut late: u128 = 0; + for entry in log.iter() { + if entry.seq > created_seq { + late = late.saturating_add(entry.amount); + } } + balance.saturating_sub(late) } +// ── Tests ───────────────────────────────────────────────────────────────────── + #[cfg(test)] mod test; diff --git a/contracts/dao_governance/src/test.rs b/contracts/dao_governance/src/test.rs index 97e00cf88..95c7b7442 100644 --- a/contracts/dao_governance/src/test.rs +++ b/contracts/dao_governance/src/test.rs @@ -4,6 +4,8 @@ use soroban_sdk::{ Env, String, }; +const DAY: u64 = 86_400; + fn setup() -> (Env, DaoGovernanceClient<'static>, Address) { let env = Env::default(); env.mock_all_auths(); @@ -14,20 +16,46 @@ fn setup() -> (Env, DaoGovernanceClient<'static>, Address) { (env, client, admin) } +fn advance(env: &Env, secs: u64) { + env.ledger().with_mut(|l| l.timestamp += secs); +} + +/// Creates a proposal with the minimum voting period (1 day). +fn new_proposal(env: &Env, client: &DaoGovernanceClient<'_>, creator: &Address) -> u64 { + client.create_proposal( + creator, + &String::from_str(env, "P"), + &String::from_str(env, "D"), + &DAY, + ) +} + +/// Builds a proposal that passes: one voter holds 1000 credits (so quorum is +/// 200), votes +20 (cost 400 >= 200), and the proposal is finalized after its +/// deadline. Returns the proposal id and the voter. +fn passing_proposal( + env: &Env, + client: &DaoGovernanceClient<'_>, + admin: &Address, +) -> (u64, Address) { + let voter = Address::generate(env); + client.grant_credits(&voter, &1_000u128); + let pid = new_proposal(env, client, admin); + client.vote(&voter, &pid, &20i64); + advance(env, DAY + 1); + client.finalize(&pid); + (pid, voter) +} + #[test] fn quadratic_cost_deducted_correctly() { let (env, client, admin) = setup(); let voter = Address::generate(&env); client.grant_credits(&voter, &100u128); - let pid = client.create_proposal( - &admin, - &String::from_str(&env, "Test"), - &String::from_str(&env, "Desc"), - &3600, - ); + let pid = new_proposal(&env, &client, &admin); - // 3 votes → cost = 9 + // 3 votes -> cost = 9 client.vote(&voter, &pid, &3i64); assert_eq!(client.credits_of(&voter), 91); @@ -37,49 +65,52 @@ fn quadratic_cost_deducted_correctly() { } #[test] -fn proposal_passes_when_tally_positive() { +fn proposal_passes_when_quorum_met_and_tally_positive() { + let (env, client, admin) = setup(); + let (pid, _voter) = passing_proposal(&env, &client, &admin); + + let p = client.get_proposal(&pid).unwrap(); + assert_eq!(p.status, ProposalStatus::Passed); + // eta = finalize time + 48h + assert_eq!(p.eta, env.ledger().timestamp() + TIMELOCK_DELAY); +} + +#[test] +fn proposal_fails_when_tally_non_positive() { let (env, client, admin) = setup(); let voter = Address::generate(&env); client.grant_credits(&voter, &1_000u128); - let pid = client.create_proposal( - &admin, - &String::from_str(&env, "Upgrade"), - &String::from_str(&env, "Details"), - &100, - ); - client.vote(&voter, &pid, &5i64); + let pid = new_proposal(&env, &client, &admin); + client.vote(&voter, &pid, &-20i64); // cost 400 >= quorum 200, but tally < 0 - env.ledger().with_mut(|l| l.timestamp += 200); + advance(&env, DAY + 1); client.finalize(&pid); - assert_eq!( - client.get_proposal(&pid).unwrap().status, - ProposalStatus::Passed - ); + let p = client.get_proposal(&pid).unwrap(); + assert_eq!(p.status, ProposalStatus::Failed); + assert_eq!(p.eta, 0); } #[test] -fn proposal_fails_when_tally_non_positive() { +fn quorum_failure_marks_proposal_failed() { let (env, client, admin) = setup(); - let voter = Address::generate(&env); - client.grant_credits(&voter, &1_000u128); + let a = Address::generate(&env); + let b = Address::generate(&env); + client.grant_credits(&a, &1_000u128); + client.grant_credits(&b, &1_000u128); // total supply 2000 -> quorum 400 - let pid = client.create_proposal( - &admin, - &String::from_str(&env, "Bad idea"), - &String::from_str(&env, "No"), - &100, - ); - client.vote(&voter, &pid, &-4i64); + let pid = new_proposal(&env, &client, &admin); + client.vote(&a, &pid, &5i64); // cost 25 < 400, tally 5 > 0 - env.ledger().with_mut(|l| l.timestamp += 200); + advance(&env, DAY + 1); client.finalize(&pid); - assert_eq!( - client.get_proposal(&pid).unwrap().status, - ProposalStatus::Failed - ); + let p = client.get_proposal(&pid).unwrap(); + assert_eq!(p.tally, 5); + assert_eq!(p.credits_spent, 25); + assert_eq!(p.quorum_required, 400); + assert_eq!(p.status, ProposalStatus::Failed); } #[test] @@ -89,12 +120,7 @@ fn cannot_vote_twice() { let voter = Address::generate(&env); client.grant_credits(&voter, &1_000u128); - let pid = client.create_proposal( - &admin, - &String::from_str(&env, "D"), - &String::from_str(&env, "D"), - &3600, - ); + let pid = new_proposal(&env, &client, &admin); client.vote(&voter, &pid, &1i64); client.vote(&voter, &pid, &1i64); // panic } @@ -106,49 +132,160 @@ fn insufficient_credits_rejected() { let voter = Address::generate(&env); client.grant_credits(&voter, &3u128); // only 3 credits + let pid = new_proposal(&env, &client, &admin); + client.vote(&voter, &pid, &5i64); // cost = 25 > 3 -> panic +} + +// ── Proposal threshold ──────────────────────────────────────────────────────── + +#[test] +#[should_panic(expected = "Error(Contract, #416)")] +fn below_threshold_creator_rejected() { + let (env, client, _admin) = setup(); + client.set_proposal_threshold(&500u128); + assert_eq!(client.get_proposal_threshold(), 500); + + let creator = Address::generate(&env); + client.grant_credits(&creator, &499u128); + new_proposal(&env, &client, &creator); +} + +#[test] +fn threshold_met_allows_creation() { + let (env, client, _admin) = setup(); + client.set_proposal_threshold(&500u128); + + let creator = Address::generate(&env); + client.grant_credits(&creator, &500u128); + let pid = new_proposal(&env, &client, &creator); + assert_eq!(client.get_proposal(&pid).unwrap().creator, creator); +} + +// ── Voting period bounds ────────────────────────────────────────────────────── + +#[test] +fn zero_duration_uses_default_seven_day_period() { + let (env, client, admin) = setup(); + assert_eq!(client.get_voting_period(), 604_800); + assert_eq!(DEFAULT_VOTING_PERIOD, 604_800); + let pid = client.create_proposal( &admin, - &String::from_str(&env, "Big"), - &String::from_str(&env, "Big"), - &3600, + &String::from_str(&env, "Default"), + &String::from_str(&env, "Default period"), + &0, ); - client.vote(&voter, &pid, &5i64); // cost = 25 > 3 → panic + let p = client.get_proposal(&pid).unwrap(); + assert_eq!(p.deadline, env.ledger().timestamp() + 604_800); } -// ── Additional tests ────────────────────────────────────────────────────────── - -/// Proposal with zero duration should panic with InvalidDeadline (#410). #[test] -#[should_panic(expected = "Error(Contract, #410)")] -fn zero_duration_proposal_rejected() { +fn period_bounds_are_inclusive() { let (env, client, admin) = setup(); - // duration = 0 → InvalidDeadline - client.create_proposal( + let min_id = client.create_proposal( &admin, - &String::from_str(&env, "ZeroDur"), - &String::from_str(&env, "Should fail"), - &0, + &String::from_str(&env, "Min"), + &String::from_str(&env, "Min"), + &MIN_VOTING_PERIOD, + ); + let max_id = client.create_proposal( + &admin, + &String::from_str(&env, "Max"), + &String::from_str(&env, "Max"), + &MAX_VOTING_PERIOD, + ); + assert_eq!( + client.get_proposal(&min_id).unwrap().deadline, + MIN_VOTING_PERIOD + ); + assert_eq!( + client.get_proposal(&max_id).unwrap().deadline, + MAX_VOTING_PERIOD ); } -/// execute() only works once a proposal has status Passed; -/// calling it on an Active proposal must panic with ProposalClosed (#405). #[test] -#[should_panic(expected = "Error(Contract, #405)")] -fn execute_on_active_proposal_rejected() { +#[should_panic(expected = "Error(Contract, #417)")] +fn duration_below_min_rejected() { + let (env, client, admin) = setup(); + new_proposal_with(&env, &client, &admin, MIN_VOTING_PERIOD - 1); +} + +#[test] +#[should_panic(expected = "Error(Contract, #417)")] +fn duration_above_max_rejected() { let (env, client, admin) = setup(); + new_proposal_with(&env, &client, &admin, MAX_VOTING_PERIOD + 1); +} + +#[test] +fn set_voting_period_changes_default() { + let (env, client, admin) = setup(); + client.set_voting_period(&(2 * DAY)); + assert_eq!(client.get_voting_period(), 2 * DAY); let pid = client.create_proposal( &admin, - &String::from_str(&env, "Active"), - &String::from_str(&env, "Still active"), - &3600, + &String::from_str(&env, "Two days"), + &String::from_str(&env, "Two days"), + &0, ); - // Attempt to execute while still Active — must panic + assert_eq!(client.get_proposal(&pid).unwrap().deadline, 2 * DAY); +} + +#[test] +#[should_panic(expected = "Error(Contract, #417)")] +fn set_voting_period_out_of_bounds_rejected() { + let (_env, client, _admin) = setup(); + client.set_voting_period(&(DAY - 1)); +} + +fn new_proposal_with(env: &Env, client: &DaoGovernanceClient<'_>, creator: &Address, d: u64) { + client.create_proposal( + creator, + &String::from_str(env, "B"), + &String::from_str(env, "B"), + &d, + ); +} + +// ── Timelock ────────────────────────────────────────────────────────────────── + +#[test] +#[should_panic(expected = "Error(Contract, #418)")] +fn timelock_blocks_early_execute() { + let (env, client, admin) = setup(); + let (pid, _voter) = passing_proposal(&env, &client, &admin); + assert_eq!( + client.get_proposal(&pid).unwrap().status, + ProposalStatus::Passed + ); + // Only ~1 day has passed since queueing; 48h timelock not elapsed. + client.execute(&admin, &pid); +} + +#[test] +fn execute_after_timelock_succeeds() { + let (env, client, admin) = setup(); + let (pid, _voter) = passing_proposal(&env, &client, &admin); + + advance(&env, TIMELOCK_DELAY); + client.execute(&admin, &pid); + + assert_eq!( + client.get_proposal(&pid).unwrap().status, + ProposalStatus::Executed + ); +} + +#[test] +#[should_panic(expected = "Error(Contract, #405)")] +fn execute_on_active_proposal_rejected() { + let (env, client, admin) = setup(); + let pid = new_proposal(&env, &client, &admin); client.execute(&admin, &pid); } -/// execute() on a Failed proposal must also panic with ProposalClosed (#405). #[test] #[should_panic(expected = "Error(Contract, #405)")] fn execute_on_failed_proposal_rejected() { @@ -156,59 +293,164 @@ fn execute_on_failed_proposal_rejected() { let voter = Address::generate(&env); client.grant_credits(&voter, &1_000u128); - let pid = client.create_proposal( - &admin, - &String::from_str(&env, "Fails"), - &String::from_str(&env, "Negative tally"), - &100, - ); + let pid = new_proposal(&env, &client, &admin); client.vote(&voter, &pid, &-3i64); - env.ledger().with_mut(|l| l.timestamp += 200); + advance(&env, DAY + 1); client.finalize(&pid); assert_eq!( client.get_proposal(&pid).unwrap().status, ProposalStatus::Failed ); - // Attempt to execute a Failed proposal — must panic client.execute(&admin, &pid); } -/// Full lifecycle: create → vote (multiple voters) → finalize → execute. +// ── Snapshot / flash-loan protection ────────────────────────────────────────── + +#[test] +#[should_panic(expected = "Error(Contract, #408)")] +fn credits_granted_after_creation_add_no_voting_power() { + let (env, client, admin) = setup(); + let voter = Address::generate(&env); + client.grant_credits(&voter, &10u128); + + let pid = new_proposal(&env, &client, &admin); + + // Flash-loan style inflation after creation. + client.grant_credits(&voter, &1_000u128); + + // Balance is 1010, but only the 10 pre-creation credits are spendable here. + // cost = 25 > 10 -> panic + client.vote(&voter, &pid, &5i64); +} + +#[test] +fn pre_creation_credits_remain_spendable_after_late_grant() { + let (env, client, admin) = setup(); + let voter = Address::generate(&env); + client.grant_credits(&voter, &10u128); + + let pid = new_proposal(&env, &client, &admin); + client.grant_credits(&voter, &1_000u128); + + client.vote(&voter, &pid, &3i64); // cost 9 <= 10 + assert_eq!(client.credits_of(&voter), 1_001); + assert_eq!(client.get_proposal(&pid).unwrap().credits_spent, 9); +} + +#[test] +fn total_granted_tracks_circulating_supply() { + let (env, client, _admin) = setup(); + let a = Address::generate(&env); + let b = Address::generate(&env); + client.grant_credits(&a, &300u128); + client.grant_credits(&b, &700u128); + assert_eq!(client.total_granted(), 1_000); +} + +// ── Treasury / ragequit ─────────────────────────────────────────────────────── + +#[test] +fn test_ragequit_dissenting_voter_burns_shares_and_receives_pro_rata() { + let (env, client, admin) = setup(); + let voter_no = Address::generate(&env); + let voter_yes = Address::generate(&env); + + client.grant_credits(&voter_no, &1_000u128); + client.grant_credits(&voter_yes, &1_000u128); // supply 2000 -> quorum 400 + + // 1000 shares total (400 for voter_no, 600 for voter_yes), 50,000 reserves + client.grant_shares(&voter_no, &400u128); + client.grant_shares(&voter_yes, &600u128); + client.deposit_treasury(&admin, &50_000u128); + + let pid = new_proposal(&env, &client, &admin); + + client.vote(&voter_no, &pid, &-2i64); // cost 4 + client.vote(&voter_yes, &pid, &20i64); // cost 400; total spent 404 >= 400 + + advance(&env, DAY + 1); + client.finalize(&pid); + assert_eq!( + client.get_proposal(&pid).unwrap().status, + ProposalStatus::Passed + ); + + // Expected payout: 400 * 50,000 / 1000 = 20,000 + let payout = client.ragequit(&voter_no, &pid, &400u128); + assert_eq!(payout, 20_000u128); + + assert_eq!(client.treasury_shares_of(&voter_no), 0); + let treasury = client.get_treasury_state(); + assert_eq!(treasury.total_shares, 600); + assert_eq!(treasury.total_reserves, 30_000); +} + +#[test] +#[should_panic(expected = "Error(Contract, #413)")] +fn test_ragequit_prohibits_yes_voters() { + let (env, client, admin) = setup(); + let (pid, voter_yes) = passing_proposal(&env, &client, &admin); + client.grant_shares(&voter_yes, &500u128); + client.deposit_treasury(&admin, &50_000u128); + + // voter_yes voted +20 (Yes): must be rejected with YesVoterCannotRagequit (413) + client.ragequit(&voter_yes, &pid, &100u128); +} + +// ── SEP-0010 nonce ──────────────────────────────────────────────────────────── + +#[test] +fn test_vote_sep0010_and_replay_rejection() { + let (env, client, admin) = setup(); + let voter = Address::generate(&env); + client.grant_credits(&voter, &100u128); + + let pid = new_proposal(&env, &client, &admin); + + let nonce = 9999u64; + let domain = String::from_str(&env, "governance.studentlab.stellar.org"); + + client.vote_sep0010(&voter, &pid, &3i64, &nonce, &domain); + + let prop = client.get_proposal(&pid).unwrap(); + assert_eq!(prop.tally, 3); + assert_eq!(client.credits_of(&voter), 100 - 9); + + // Replay with same nonce is rejected + let res = client.try_vote_sep0010(&voter, &pid, &1i64, &nonce, &domain); + assert!(res.is_err()); +} + +// ── Lifecycle, tally, and views ─────────────────────────────────────────────── + #[test] -fn execute_passed_proposal_succeeds() { +fn execute_passed_proposal_full_lifecycle() { let (env, client, admin) = setup(); let voter1 = Address::generate(&env); let voter2 = Address::generate(&env); client.grant_credits(&voter1, &500u128); - client.grant_credits(&voter2, &500u128); + client.grant_credits(&voter2, &500u128); // supply 1000 -> quorum 200 - let pid = client.create_proposal( - &admin, - &String::from_str(&env, "Upgrade"), - &String::from_str(&env, "Details"), - &100, - ); - client.vote(&voter1, &pid, &4i64); // cost = 16 - client.vote(&voter2, &pid, &3i64); // cost = 9 + let pid = new_proposal(&env, &client, &admin); + client.vote(&voter1, &pid, &10i64); // cost 100 + client.vote(&voter2, &pid, &10i64); // cost 100; spent 200 >= 200 - env.ledger().with_mut(|l| l.timestamp += 200); + advance(&env, DAY + 1); client.finalize(&pid); - assert_eq!( client.get_proposal(&pid).unwrap().status, ProposalStatus::Passed ); + advance(&env, TIMELOCK_DELAY); client.execute(&admin, &pid); - assert_eq!( client.get_proposal(&pid).unwrap().status, ProposalStatus::Executed ); } -/// Quorum simulation: multiple voters cast votes; final tally aggregates correctly. #[test] fn multiple_voter_tally_aggregates_correctly() { let (env, client, admin) = setup(); @@ -220,32 +462,25 @@ fn multiple_voter_tally_aggregates_correctly() { let opponents = [Address::generate(&env), Address::generate(&env)]; for addr in supporters.iter().chain(opponents.iter()) { - client.grant_credits(addr, &1_000u128); + client.grant_credits(addr, &1_000u128); // supply 5000 -> quorum 1000 } - let pid = client.create_proposal( - &admin, - &String::from_str(&env, "Quorum test"), - &String::from_str(&env, "Many voters"), - &500, - ); + let pid = new_proposal(&env, &client, &admin); - // 3 supporters each cast 2 votes (+2 tally each, cost = 4 each) + // 3 supporters x (+20, cost 400) = tally +60, spent 1200 for addr in &supporters { - client.vote(addr, &pid, &2i64); + client.vote(addr, &pid, &20i64); } - // 2 opponents each cast -1 vote (−1 tally each, cost = 1 each) + // 2 opponents x (-1, cost 1) = tally -2, spent 2 for addr in &opponents { client.vote(addr, &pid, &-1i64); } let p = client.get_proposal(&pid).unwrap(); - // tally = 3×2 − 2×1 = 4 - assert_eq!(p.tally, 4); - // credits_spent = 3×4 + 2×1 = 14 - assert_eq!(p.credits_spent, 14); + assert_eq!(p.tally, 58); + assert_eq!(p.credits_spent, 1_202); - env.ledger().with_mut(|l| l.timestamp += 600); + advance(&env, DAY + 1); client.finalize(&pid); assert_eq!( client.get_proposal(&pid).unwrap().status, @@ -253,64 +488,42 @@ fn multiple_voter_tally_aggregates_correctly() { ); } -/// finalize() before deadline must panic with VotingDeadlineLive (#406). #[test] #[should_panic(expected = "Error(Contract, #406)")] fn finalize_before_deadline_panics() { let (env, client, admin) = setup(); - - let pid = client.create_proposal( - &admin, - &String::from_str(&env, "TooEarly"), - &String::from_str(&env, "Not yet"), - &3600, - ); - // Ledger timestamp is still 0, deadline is 3600 — finalize must panic + let pid = new_proposal(&env, &client, &admin); + // Timestamp is still 0, deadline is 1 day - finalize must panic client.finalize(&pid); } -/// get_proposal for an unknown ID returns None (does not panic). #[test] fn get_proposal_returns_none_for_unknown_id() { let (_env, client, _admin) = setup(); assert!(client.get_proposal(&999u64).is_none()); } -/// vote_of returns the correct vote after casting. #[test] fn vote_of_returns_correct_value() { let (env, client, admin) = setup(); let voter = Address::generate(&env); client.grant_credits(&voter, &1_000u128); - let pid = client.create_proposal( - &admin, - &String::from_str(&env, "VoteOf"), - &String::from_str(&env, "Check vote stored"), - &3600, - ); + let pid = new_proposal(&env, &client, &admin); client.vote(&voter, &pid, &-2i64); - let stored = client.vote_of(&pid, &voter); - assert_eq!(stored, Some(-2i64)); + assert_eq!(client.vote_of(&pid, &voter), Some(-2i64)); } -/// vote_of returns None when the voter has not yet cast a vote. #[test] fn vote_of_returns_none_before_voting() { let (env, client, admin) = setup(); let non_voter = Address::generate(&env); - let pid = client.create_proposal( - &admin, - &String::from_str(&env, "NoVote"), - &String::from_str(&env, "Unvoted"), - &3600, - ); + let pid = new_proposal(&env, &client, &admin); assert!(client.vote_of(&pid, &non_voter).is_none()); } -/// Credits accumulate correctly when grant_credits is called multiple times. #[test] fn grant_credits_accumulates() { let (env, client, _admin) = setup(); @@ -321,20 +534,14 @@ fn grant_credits_accumulates() { assert_eq!(client.credits_of(&member), 150u128); } -/// After voting the voter's credit balance decreases by votes². #[test] fn credits_decrease_by_quadratic_cost_on_negative_vote() { let (env, client, admin) = setup(); let voter = Address::generate(&env); client.grant_credits(&voter, &200u128); - let pid = client.create_proposal( - &admin, - &String::from_str(&env, "Neg"), - &String::from_str(&env, "Against"), - &3600, - ); - // −4 votes → cost = 16 + let pid = new_proposal(&env, &client, &admin); + // -4 votes -> cost 16 client.vote(&voter, &pid, &-4i64); assert_eq!(client.credits_of(&voter), 184u128); let p = client.get_proposal(&pid).unwrap(); From 2ede49646844cde5430eb44667e06751e27414e1 Mon Sep 17 00:00:00 2001 From: GitHub Copilot Date: Sat, 17 Oct 2026 16:29:06 +0100 Subject: [PATCH 4/4] feat(quadratic): overflow-safe isqrt, sybil gate, commit-reveal, exact pool split (#1626) - math.rs: checked isqrt and quadratic cost - Funding checks sybil_resistance is_verified when configured - Commit-reveal donation rounds - Distribution assigns rounding remainder so payouts equal the pool --- contracts/quadratic_funding/src/lib.rs | 349 ++++++++++++++++++++---- contracts/quadratic_funding/src/test.rs | 206 +++++++++++++- contracts/quadratic_voting/src/lib.rs | 46 +++- contracts/quadratic_voting/src/math.rs | 66 +++++ contracts/quadratic_voting/src/test.rs | 68 +++++ 5 files changed, 666 insertions(+), 69 deletions(-) create mode 100644 contracts/quadratic_voting/src/math.rs diff --git a/contracts/quadratic_funding/src/lib.rs b/contracts/quadratic_funding/src/lib.rs index 95138fe62..cbc19ccb5 100644 --- a/contracts/quadratic_funding/src/lib.rs +++ b/contracts/quadratic_funding/src/lib.rs @@ -1,6 +1,6 @@ #![no_std] -//! # Quadratic Funding & Voting Allocation Engine — SC-HARD-14 +//! # Quadratic Funding & Voting Allocation Engine — SC-HARD-14 / SC-005 //! //! A Gitcoin-style quadratic funding (QF) mechanism where a central matching //! pool is distributed to projects proportional to the *square* of the sum of @@ -14,29 +14,43 @@ //! match(p) = (Σ sqrt(a_i))² //! ``` //! -//! The matching pool is then distributed proportionally: +//! The matching pool is then distributed proportionally, with floor rounding +//! and any dust (the rounding remainder) assigned to the project with the +//! largest weight so that payouts always sum to exactly the pool: //! //! ```text -//! payout(p) = match(p) / Σ_all_projects match(q) × pool +//! payout(p) = floor(match(p) / Σ_all_projects match(q) × pool) //! ``` //! //! ## Sybil Resistance -//! Each address may donate to a given project **at most once** per round. -//! The admin maintains a whitelist of approved donors; only whitelisted -//! addresses may contribute. +//! - Each address may donate to a given project **at most once** per round. +//! - Donors must be on the admin whitelist. +//! - If a sybil-resistance contract is configured via `set_sybil_contract`, +//! every donor must also pass `is_verified(donor)` on that contract. +//! +//! ## Commit-reveal (anti-frontrunning) +//! 1. Commit phase (default): donors call `commit_donation` with a hash. +//! 2. Admin calls `open_reveal`, closing the commit phase. +//! 3. Reveal phase: donors call `reveal_donation` with the preimage fields. +//! The commitment is checked and the donation is applied via the same +//! internal path as `donate`. +//! +//! Commitment preimage (see [`commitment_hash`]): +//! `"QF-COMMIT-V1" || donor_xdr || project_id(u64, BE) || amount(i128, BE) || salt(32 bytes)` //! //! ## Security //! - Integer overflow: all arithmetic uses `checked_*`. -//! - Precision: square-root is computed with integer Newton's method scaled -//! by `SCALE = 1_000_000` to preserve 6 decimal places of precision. -//! - No external calls during state mutation. +//! - Precision: square-root is computed with integer arithmetic scaled by +//! `SCALE = 1_000_000` to preserve 6 decimal places of precision. +//! - Errors are reported with panics and clear messages. //! //! ## Storage TTL (SC-HARD-16) //! Every state-mutating call bumps instance TTL beyond 100,000 ledgers to //! protect against automatic ledger state archival on Stellar Mainnet. use soroban_sdk::{ - contract, contractimpl, contracttype, symbol_short, Address, Env, Map, Symbol, Vec, + contract, contractimpl, contracttype, symbol_short, xdr::ToXdr, Address, Bytes, BytesN, Env, + IntoVal, Map, Symbol, Vec, }; // ── TTL constants (SC-HARD-16) ─────────────────────────────────────────────── @@ -51,6 +65,11 @@ const PROJECTS_KEY: Symbol = symbol_short!("PROJECTS"); const WHITELIST_KEY: Symbol = symbol_short!("WLIST"); const ROUND_OPEN: Symbol = symbol_short!("OPEN"); const ADMIN_KEY: Symbol = symbol_short!("ADMIN"); +const SYBIL_KEY: Symbol = symbol_short!("SYBIL"); +const REVEAL_OPEN: Symbol = symbol_short!("REVEAL"); + +/// Domain separator prepended to every commitment preimage. +const COMMIT_DOMAIN: &[u8] = b"QF-COMMIT-V1"; /// Fixed-point scale for sqrt precision (6 decimal places). const SCALE: i128 = 1_000_000; @@ -71,6 +90,42 @@ pub struct Project { pub donors: Map, } +/// Persistent keys for per-(donor, project) commitments. +#[contracttype] +#[derive(Clone)] +pub enum QfKey { + /// Commitment hash stored by `commit_donation`. + Commit(Address, u32), +} + +// ── Commitment helper ───────────────────────────────────────────────────────── + +/// Builds the commit-reveal hash for a donation. +/// +/// Preimage layout (byte-exact): +/// 1. `"QF-COMMIT-V1"` (12 bytes, ASCII domain separator) +/// 2. donor address, XDR-serialised (variable length) +/// 3. `project_id` as `u64`, big-endian (8 bytes) +/// 4. `amount` as `i128`, big-endian (16 bytes) +/// 5. `salt` (32 bytes) +/// +/// The fields after the donor XDR have fixed lengths, so the encoding is +/// unambiguous. Returns `sha256(preimage)`. +pub fn commitment_hash( + env: &Env, + donor: &Address, + project_id: u32, + amount: i128, + salt: &BytesN<32>, +) -> BytesN<32> { + let mut preimage = Bytes::from_slice(env, COMMIT_DOMAIN); + preimage.append(&donor.clone().to_xdr(env)); + preimage.append(&Bytes::from_array(env, &u64::from(project_id).to_be_bytes())); + preimage.append(&Bytes::from_array(env, &amount.to_be_bytes())); + preimage.append(&Bytes::from_array(env, &salt.to_array())); + env.crypto().sha256(&preimage).to_bytes() +} + // ── Contract ────────────────────────────────────────────────────────────────── #[contract] @@ -82,6 +137,8 @@ impl QuadraticFunding { /// Initialise a new funding round with a matching `pool`. /// + /// The round starts in the commit phase (reveal not open). + /// /// # Panics /// - If a round is already open. /// - If `pool` is not positive. @@ -100,10 +157,25 @@ impl QuadraticFunding { env.storage() .instance() .set(&WHITELIST_KEY, &Map::::new(&env)); + env.storage().instance().set(&REVEAL_OPEN, &false); env.storage().instance().set(&ROUND_OPEN, &true); bump_instance(&env); } + // ── Admin: sybil contract ───────────────────────────────────────────────── + + /// Configure the sybil-resistance contract consulted by `donate` and + /// `reveal_donation`. The contract must expose `is_verified(Address) -> bool`. + /// + /// # Panics + /// - If `admin` is not the round admin. + pub fn set_sybil_contract(env: Env, admin: Address, sybil: Address) { + admin.require_auth(); + Self::assert_admin(&env, &admin); + env.storage().instance().set(&SYBIL_KEY, &sybil); + bump_instance(&env); + } + // ── Admin: register project ─────────────────────────────────────────────── /// Register a new project with id `project_id`. @@ -154,63 +226,119 @@ impl QuadraticFunding { /// # Panics /// - If the round is not open. /// - If `donor` is not whitelisted. + /// - If a sybil contract is configured and `donor` is not sybil-verified. /// - If `donor` has already donated to this project. /// - If `amount` is not positive. pub fn donate(env: Env, donor: Address, project_id: u32, amount: i128) { + donor.require_auth(); + Self::process_donation(&env, &donor, project_id, amount); + } + + // ── Commit-reveal ───────────────────────────────────────────────────────── + + /// Commit to a future donation. Only allowed during the commit phase + /// (before `open_reveal`). The commitment is `commitment_hash(...)`. + /// + /// # Panics + /// - If the round is not open or the reveal phase has started. + /// - If the project does not exist. + /// - If `donor` already has a commitment for `project_id`. + pub fn commit_donation(env: Env, donor: Address, project_id: u32, commitment: BytesN<32>) { donor.require_auth(); Self::assert_round_open(&env); - assert!(amount > 0, "amount must be positive"); + assert!(!Self::is_reveal_open(&env), "commit phase closed"); - // Sybil check - let wl: Map = env.storage().instance().get(&WHITELIST_KEY).unwrap(); + let projects: Map = env.storage().instance().get(&PROJECTS_KEY).unwrap(); + assert!(projects.contains_key(project_id), "project not found"); + + let key = QfKey::Commit(donor.clone(), project_id); assert!( - wl.get(donor.clone()).unwrap_or(false), - "donor not whitelisted" + !env.storage().persistent().has(&key), + "already committed to this project" ); + env.storage().persistent().set(&key, &commitment); + env.storage() + .persistent() + .extend_ttl(&key, TTL_THRESHOLD, INSTANCE_BUMP); - let mut projects: Map = env.storage().instance().get(&PROJECTS_KEY).unwrap(); - let mut project = projects.get(project_id).expect("project not found"); - - // One donation per donor per project (Sybil guard). - assert!( - !project.donors.contains_key(donor.clone()), - "already donated to this project" + env.events().publish( + (symbol_short!("committed"), donor.clone()), + project_id, ); + bump_instance(&env); + } - project.donors.set(donor.clone(), amount); - project.total_donations = project - .total_donations - .checked_add(amount) - .expect("overflow"); + /// Close the commit phase and open the reveal phase. Admin only. + /// + /// # Panics + /// - If `admin` is not the round admin. + /// - If the round is not open. + /// - If the reveal phase is already open. + pub fn open_reveal(env: Env, admin: Address) { + admin.require_auth(); + Self::assert_admin(&env, &admin); + Self::assert_round_open(&env); + assert!(!Self::is_reveal_open(&env), "reveal already open"); - // sqrt_sum += isqrt(amount * SCALE²) = isqrt(amount) * SCALE - let sqrt_contribution = Self::isqrt(amount.checked_mul(SCALE * SCALE).expect("overflow")); - project.sqrt_sum = project - .sqrt_sum - .checked_add(sqrt_contribution) - .expect("overflow"); + env.storage().instance().set(&REVEAL_OPEN, &true); + bump_instance(&env); + } - projects.set(project_id, project); - env.storage().instance().set(&PROJECTS_KEY, &projects); + /// Reveal a previously committed donation and apply it. + /// + /// The hash of `(donor, project_id, amount, salt)` must match the stored + /// commitment. The commitment is consumed on success, so it cannot be replayed. + /// + /// # Panics + /// - If the round is not open or the reveal phase is not open. + /// - If there is no commitment for `(donor, project_id)`. + /// - If the recomputed hash does not match the commitment (wrong salt/amount). + /// - Any check performed by the donation path (whitelist, sybil, duplicate, amount). + pub fn reveal_donation( + env: Env, + donor: Address, + project_id: u32, + amount: i128, + salt: BytesN<32>, + ) { + donor.require_auth(); + Self::assert_round_open(&env); + assert!(Self::is_reveal_open(&env), "reveal phase not open"); - env.events() - .publish((symbol_short!("donated"), donor), (project_id, amount)); - bump_instance(&env); + let key = QfKey::Commit(donor.clone(), project_id); + let committed: BytesN<32> = env + .storage() + .persistent() + .get(&key) + .expect("no commitment for this donor and project"); + + let digest = commitment_hash(&env, &donor, project_id, amount, &salt); + assert!(digest == committed, "commitment mismatch"); + + env.storage().persistent().remove(&key); + Self::process_donation(&env, &donor, project_id, amount); } // ── Distribute ──────────────────────────────────────────────────────────── /// Close the round and distribute the matching pool to projects. /// - /// Returns a `Vec` of `(project_id, match_amount)` pairs. + /// Returns a `Vec` of `(project_id, match_amount)` pairs, in project-id order. + /// + /// Each project receives `floor(pool × weight / total_weight)`. The rounding + /// remainder (dust) is added to the project with the largest weight; ties go + /// to the lowest project id. The payouts therefore always sum to exactly the + /// pool. If no project has a non-zero weight, no payouts are made and the + /// round still closes. /// /// The quadratic formula ensures that projects with **more unique donors** /// receive proportionally more matching, even if their total donation amount - /// is equal to a project with fewer whale donors. This maximises capital - /// allocation toward community-favored public goods. + /// is equal to a project with fewer whale donors. /// /// # Panics + /// - If `admin` is not the round admin. /// - If the round is not open. + /// - On arithmetic overflow. pub fn distribute(env: Env, admin: Address) -> Vec<(u32, i128)> { admin.require_auth(); Self::assert_admin(&env, &admin); @@ -219,12 +347,11 @@ impl QuadraticFunding { let pool: i128 = env.storage().instance().get(&POOL_KEY).unwrap(); let projects: Map = env.storage().instance().get(&PROJECTS_KEY).unwrap(); - // Compute match weight for each project: (sqrt_sum)² + // Compute match weight for each project: (sqrt_sum)² / SCALE². let mut total_weight: i128 = 0; let mut weights: Vec<(u32, i128)> = Vec::new(&env); for (id, project) in projects.iter() { - // weight = (sqrt_sum)² / SCALE² normalises back to token units. let weight = project .sqrt_sum .checked_mul(project.sqrt_sum) @@ -244,16 +371,36 @@ impl QuadraticFunding { return payouts; } + // Pass 1: floor shares; track total distributed and the largest weight. + let mut distributed: i128 = 0; + let mut largest_idx: u32 = 0; + let mut largest_weight: i128 = -1; for i in 0..weights.len() { let (id, weight) = weights.get(i).unwrap(); - let match_amount = pool + let share = pool .checked_mul(weight) .expect("overflow") .checked_div(total_weight) - .unwrap_or(0); - payouts.push_back((id, match_amount)); - env.events() - .publish((symbol_short!("payout"),), (id, match_amount)); + .expect("division by zero"); + distributed = distributed.checked_add(share).expect("overflow"); + if weight > largest_weight { + largest_weight = weight; + largest_idx = i; + } + payouts.push_back((id, share)); + } + + // Pass 2: assign rounding dust to the largest-weight project. + // distributed <= pool because every floor share is <= its exact share. + let remainder = pool.checked_sub(distributed).expect("overflow"); + if remainder > 0 { + let (id, share) = payouts.get(largest_idx).unwrap(); + let adjusted = share.checked_add(remainder).expect("overflow"); + payouts.set(largest_idx, (id, adjusted)); + } + + for (id, amount) in payouts.iter() { + env.events().publish((symbol_short!("payout"),), (id, amount)); } // Close the round. @@ -276,6 +423,11 @@ impl QuadraticFunding { env.storage().instance().get(&POOL_KEY).unwrap_or(0) } + /// Returns `true` once the reveal phase has been opened. + pub fn is_reveal_phase(env: Env) -> bool { + Self::is_reveal_open(&env) + } + // ── SC-037 Penalty Slasher & Collusion Detection ────────────────────────── /// Deposit security stake for a donor. @@ -307,20 +459,104 @@ impl QuadraticFunding { // ── Internal helpers ────────────────────────────────────────────────────── - /// Integer square root via Newton's method (no floating point). + /// Shared donation path used by `donate` and `reveal_donation`. + /// Performs no authentication; callers must `require_auth` first. + fn process_donation(env: &Env, donor: &Address, project_id: u32, amount: i128) { + Self::assert_round_open(env); + assert!(amount > 0, "amount must be positive"); + + // Whitelist check. + let wl: Map = env.storage().instance().get(&WHITELIST_KEY).unwrap(); + assert!( + wl.get(donor.clone()).unwrap_or(false), + "donor not whitelisted" + ); + + // Sybil-resistance contract check (when configured). + Self::check_sybil(env, donor); + + let mut projects: Map = env.storage().instance().get(&PROJECTS_KEY).unwrap(); + let mut project = projects.get(project_id).expect("project not found"); + + // One donation per donor per project (Sybil guard). + assert!( + !project.donors.contains_key(donor.clone()), + "already donated to this project" + ); + + project.donors.set(donor.clone(), amount); + project.total_donations = project + .total_donations + .checked_add(amount) + .expect("overflow"); + + // sqrt_sum += isqrt(amount * SCALE²) = isqrt(amount) * SCALE + let scaled = amount + .checked_mul(SCALE) + .and_then(|v| v.checked_mul(SCALE)) + .expect("overflow"); + let sqrt_contribution = Self::isqrt(scaled); + project.sqrt_sum = project + .sqrt_sum + .checked_add(sqrt_contribution) + .expect("overflow"); + + projects.set(project_id, project); + env.storage().instance().set(&PROJECTS_KEY, &projects); + + env.events() + .publish((symbol_short!("donated"), donor.clone()), (project_id, amount)); + bump_instance(env); + } + + /// Calls `is_verified(donor)` on the configured sybil contract, if any. + /// Panics if the donor is not verified. Does nothing when no contract is set. + fn check_sybil(env: &Env, donor: &Address) { + let sybil: Option
= env.storage().instance().get(&SYBIL_KEY); + if let Some(sybil_contract) = sybil { + let verified: bool = env.invoke_contract( + &sybil_contract, + &Symbol::new(env, "is_verified"), + soroban_sdk::vec![env, donor.into_val(env)], + ); + assert!(verified, "donor not sybil verified"); + } + } + + /// Integer square root via the bitwise digit method (no floating point, + /// no unchecked `x + 1` or `x + n / x` steps). /// - /// Returns `floor(sqrt(n))`. + /// Returns `floor(sqrt(n))`, or `0` for `n <= 0`. pub(crate) fn isqrt(n: i128) -> i128 { if n <= 0 { return 0; } - let mut x = n; - let mut y = (x + 1) / 2; - while y < x { - x = y; - y = (x + n / x) / 2; + let mut num = n as u128; + let mut res: u128 = 0; + // Highest power of four <= num. Every intermediate stays below 2^127. + let mut bit: u128 = 1u128 << 126; + while bit > num { + bit >>= 2; + } + while bit != 0 { + let candidate = res.checked_add(bit).expect("overflow"); + if num >= candidate { + num = num.checked_sub(candidate).expect("overflow"); + res = (res >> 1).checked_add(bit).expect("overflow"); + } else { + res >>= 1; + } + bit >>= 2; } - x + // The root of a positive i128 is at most 2^63, so this cast is lossless. + res as i128 + } + + fn is_reveal_open(env: &Env) -> bool { + env.storage() + .instance() + .get::(&REVEAL_OPEN) + .unwrap_or(false) } fn assert_admin(env: &Env, caller: &Address) { @@ -359,4 +595,3 @@ pub(crate) fn bump_instance(env: &Env) { #[cfg(test)] mod test; - diff --git a/contracts/quadratic_funding/src/test.rs b/contracts/quadratic_funding/src/test.rs index bb58d7f7d..ea385d35c 100644 --- a/contracts/quadratic_funding/src/test.rs +++ b/contracts/quadratic_funding/src/test.rs @@ -13,7 +13,29 @@ use super::*; use soroban_sdk::testutils::Address as _; -use soroban_sdk::{Bytes, Env}; +use soroban_sdk::{Bytes, BytesN, Env}; + +// ── Mock sybil-resistance contracts ─────────────────────────────────────────── + +#[contract] +struct MockSybilOk; + +#[contractimpl] +impl MockSybilOk { + pub fn is_verified(_env: Env, _user: Address) -> bool { + true + } +} + +#[contract] +struct MockSybilReject; + +#[contractimpl] +impl MockSybilReject { + pub fn is_verified(_env: Env, _user: Address) -> bool { + false + } +} // ── Helper ──────────────────────────────────────────────────────────────────── @@ -363,3 +385,185 @@ fn test_slash_collusion_rejects_non_guilty_donor() { let res = client.try_slash_collusion(&investigator, &1, &colluders, &proof); assert_eq!(res, Err(Ok(SlasherError::DonorNotGuilty))); } + +// ── SC-005 tests ────────────────────────────────────────────────────────────── + +#[test] +fn test_isqrt_boundaries_and_max() { + assert_eq!(QuadraticFunding::isqrt(0), 0); + assert_eq!(QuadraticFunding::isqrt(-7), 0); + assert_eq!(QuadraticFunding::isqrt(3), 1); + assert_eq!(QuadraticFunding::isqrt(99), 9); + assert_eq!(QuadraticFunding::isqrt(100), 10); + // Must not panic at the type boundary; floor(sqrt(MAX)) ≈ 1.3e19. + let r = QuadraticFunding::isqrt(i128::MAX); + assert!(r.checked_mul(r).unwrap() <= i128::MAX); +} + +#[test] +fn test_sybil_unverified_donor_rejected_in_donate() { + let (env, contract_id, admin) = setup(); + let client = QuadraticFundingClient::new(&env, &contract_id); + let donor = Address::generate(&env); + let owner = Address::generate(&env); + let sybil = env.register(MockSybilReject, ()); + + client.init(&admin, &10_000); + client.set_sybil_contract(&admin, &sybil); + client.register_project(&admin, &1, &owner); + client.whitelist_donor(&admin, &donor); + + let res = client.try_donate(&donor, &1, &100); + assert!(res.is_err(), "unverified sybil donor must be rejected"); + assert_eq!(client.get_project(&1).total_donations, 0); +} + +#[test] +fn test_sybil_verified_donor_accepted_in_donate() { + let (env, contract_id, admin) = setup(); + let client = QuadraticFundingClient::new(&env, &contract_id); + let donor = Address::generate(&env); + let owner = Address::generate(&env); + let sybil = env.register(MockSybilOk, ()); + + client.init(&admin, &10_000); + client.set_sybil_contract(&admin, &sybil); + client.register_project(&admin, &1, &owner); + client.whitelist_donor(&admin, &donor); + client.donate(&donor, &1, &100); + assert_eq!(client.get_project(&1).total_donations, 100); +} + +#[test] +fn test_distribute_sums_exactly_to_pool() { + let (env, contract_id, admin) = setup(); + let client = QuadraticFundingClient::new(&env, &contract_id); + client.init(&admin, &10_000); + for id in 1u32..=3 { + let owner = Address::generate(&env); + client.register_project(&admin, &id, &owner); + let d = Address::generate(&env); + client.whitelist_donor(&admin, &d); + client.donate(&d, &id, &100); + } + + let payouts = client.distribute(&admin); + assert_eq!(payouts.len(), 3); + let mut sum = 0i128; + for i in 0..payouts.len() { + sum += payouts.get(i).unwrap().1; + } + assert_eq!(sum, 10_000); + // Equal weights: 3333 each, remainder 1 goes to the first largest-weight project. + assert_eq!(payouts.get(0).unwrap(), (1, 3334)); + assert_eq!(payouts.get(1).unwrap(), (2, 3333)); + assert_eq!(payouts.get(2).unwrap(), (3, 3333)); +} + +#[test] +fn test_distribute_uneven_weights_sums_exactly_to_pool() { + let (env, contract_id, admin) = setup(); + let client = QuadraticFundingClient::new(&env, &contract_id); + client.init(&admin, &9_999); + let amounts = [7i128, 13, 29]; + for (idx, amount) in amounts.iter().enumerate() { + let id = (idx as u32) + 1; + let owner = Address::generate(&env); + client.register_project(&admin, &id, &owner); + let d = Address::generate(&env); + client.whitelist_donor(&admin, &d); + client.donate(&d, &id, amount); + } + + let payouts = client.distribute(&admin); + let mut sum = 0i128; + for i in 0..payouts.len() { + sum += payouts.get(i).unwrap().1; + } + assert_eq!(sum, 9_999); + // Round is closed: a second distribute must fail. + assert!(client.try_distribute(&admin).is_err()); +} + +#[test] +fn test_commit_then_reveal_succeeds() { + let (env, contract_id, admin) = setup(); + let client = QuadraticFundingClient::new(&env, &contract_id); + let donor = Address::generate(&env); + let owner = Address::generate(&env); + + client.init(&admin, &10_000); + client.register_project(&admin, &1, &owner); + client.whitelist_donor(&admin, &donor); + + let salt = BytesN::from_array(&env, &[7u8; 32]); + let hash = commitment_hash(&env, &donor, 1, 100, &salt); + client.commit_donation(&donor, &1, &hash); + + client.open_reveal(&admin); + assert!(client.is_reveal_phase()); + client.reveal_donation(&donor, &1, &100, &salt); + + let project = client.get_project(&1); + assert_eq!(project.total_donations, 100); + assert!(project.donors.contains_key(donor)); +} + +#[test] +fn test_reveal_with_wrong_salt_fails() { + let (env, contract_id, admin) = setup(); + let client = QuadraticFundingClient::new(&env, &contract_id); + let donor = Address::generate(&env); + let owner = Address::generate(&env); + + client.init(&admin, &10_000); + client.register_project(&admin, &1, &owner); + client.whitelist_donor(&admin, &donor); + + let salt = BytesN::from_array(&env, &[7u8; 32]); + let wrong_salt = BytesN::from_array(&env, &[8u8; 32]); + let hash = commitment_hash(&env, &donor, 1, 100, &salt); + client.commit_donation(&donor, &1, &hash); + client.open_reveal(&admin); + + let res = client.try_reveal_donation(&donor, &1, &100, &wrong_salt); + assert!(res.is_err(), "reveal with wrong salt must fail"); + // The commitment is still intact and the correct reveal still works. + assert_eq!(client.get_project(&1).total_donations, 0); + client.reveal_donation(&donor, &1, &100, &salt); + assert_eq!(client.get_project(&1).total_donations, 100); +} + +#[test] +fn test_commit_after_reveal_opened_fails() { + let (env, contract_id, admin) = setup(); + let client = QuadraticFundingClient::new(&env, &contract_id); + let donor = Address::generate(&env); + let owner = Address::generate(&env); + + client.init(&admin, &10_000); + client.register_project(&admin, &1, &owner); + client.whitelist_donor(&admin, &donor); + client.open_reveal(&admin); + + let salt = BytesN::from_array(&env, &[1u8; 32]); + let hash = commitment_hash(&env, &donor, 1, 100, &salt); + assert!(client.try_commit_donation(&donor, &1, &hash).is_err()); +} + +#[test] +fn test_reveal_before_open_fails() { + let (env, contract_id, admin) = setup(); + let client = QuadraticFundingClient::new(&env, &contract_id); + let donor = Address::generate(&env); + let owner = Address::generate(&env); + + client.init(&admin, &10_000); + client.register_project(&admin, &1, &owner); + client.whitelist_donor(&admin, &donor); + + let salt = BytesN::from_array(&env, &[1u8; 32]); + let hash = commitment_hash(&env, &donor, 1, 100, &salt); + client.commit_donation(&donor, &1, &hash); + assert!(client.try_reveal_donation(&donor, &1, &100, &salt).is_err()); +} diff --git a/contracts/quadratic_voting/src/lib.rs b/contracts/quadratic_voting/src/lib.rs index fbc05bcd2..1fb529155 100644 --- a/contracts/quadratic_voting/src/lib.rs +++ b/contracts/quadratic_voting/src/lib.rs @@ -6,6 +6,8 @@ #![no_std] +pub mod math; + use soroban_sdk::{ contract, contracterror, contractimpl, contracttype, panic_with_error, Address, BytesN, Env, IntoVal, String, Symbol, @@ -33,6 +35,8 @@ pub enum QVError { NotSybilVerified = 445, /// `446` — Proposal was not found. ProposalNotFound = 446, + /// `447` — Arithmetic overflow while computing votes or cost. + Overflow = 447, } #[contracttype] @@ -129,7 +133,9 @@ impl QuadraticVotingContract { .instance() .get(&DataKey::ProposalCount) .unwrap_or(0); - count += 1; + count = count + .checked_add(1) + .unwrap_or_else(|| panic_with_error!(&env, QVError::Overflow)); let proposal = Proposal { id: count, @@ -187,19 +193,33 @@ impl QuadraticVotingContract { .persistent() .get(&DataKey::UserVotes(voter.clone(), proposal_id)) .unwrap_or(0); - let new_total_votes = previous_votes + additional_votes; - - // Quadratic cost logic: Total cost should be (total_votes)^2. - let total_cost = new_total_votes.pow(2); - let previous_cost = previous_votes.pow(2); - let incremental_cost = total_cost - previous_cost; - - if current_credits < incremental_cost { + let new_total_votes = previous_votes + .checked_add(additional_votes) + .unwrap_or_else(|| panic_with_error!(&env, QVError::Overflow)); + + // Quadratic cost logic: incremental cost = (prev + add)^2 - prev^2, + // so that the total cost of V votes is V^2. + let incremental_cost = math::quadratic_cost( + u64::from(previous_votes), + u64::from(additional_votes), + ) + .unwrap_or_else(|| panic_with_error!(&env, QVError::Overflow)); + + if i128::from(current_credits) < incremental_cost { panic_with_error!(&env, QVError::InsufficientCredits); } - current_credits -= incremental_cost; - proposal.votes_received += additional_votes; + let remaining = i128::from(current_credits) + .checked_sub(incremental_cost) + .unwrap_or_else(|| panic_with_error!(&env, QVError::InsufficientCredits)); + // remaining <= current_credits <= u32::MAX, so this conversion succeeds. + current_credits = u32::try_from(remaining) + .unwrap_or_else(|_| panic_with_error!(&env, QVError::Overflow)); + + proposal.votes_received = proposal + .votes_received + .checked_add(additional_votes) + .unwrap_or_else(|| panic_with_error!(&env, QVError::Overflow)); env.storage() .persistent() @@ -280,6 +300,10 @@ impl QuadraticVotingContract { /// Returns the matching pool weight for a voter. Only voters with /// verified DID credentials (proof_count > 0) receive non-zero weight. /// Unverified voters (score = 0) always return 0 matching pool weight. + /// + /// NOTE: this weight is LINEAR in the humanity score (score * 10). It is + /// not the quadratic consensus weight used by `quadratic_funding::distribute`. + /// Kept unchanged for compatibility. pub fn get_matching_pool_weight(env: Env, voter: Address) -> u32 { let score: u32 = Self::get_humanity_score(env, voter.clone()); // Unverified account: zero matching pool weight. diff --git a/contracts/quadratic_voting/src/math.rs b/contracts/quadratic_voting/src/math.rs new file mode 100644 index 000000000..49f4b3e0d --- /dev/null +++ b/contracts/quadratic_voting/src/math.rs @@ -0,0 +1,66 @@ +//! Overflow-safe integer math for quadratic voting (SC-005). +//! +//! All helpers avoid floating point and use checked arithmetic. No helper +//! performs an unchecked `x + 1` or `x + n / x` step, so inputs at the type +//! boundary (e.g. `i128::MAX`) cannot panic. + +/// Integer square root: returns `floor(sqrt(n))` for an unsigned 128-bit value. +/// +/// Uses the classic digit-by-digit (bitwise) method. Every intermediate value +/// stays below `2^127`, so the additions below cannot overflow; the `checked_*` +/// calls are kept as a defensive guard and are never expected to fail. +pub fn isqrt_u128(n: u128) -> u128 { + if n < 2 { + return n; + } + + let mut num = n; + let mut res: u128 = 0; + + // Highest power of four that is <= n. + let mut bit: u128 = 1u128 << 126; + while bit > num { + bit >>= 2; + } + + while bit != 0 { + // res < 2^64 (it holds the already-determined bits of the root) and + // bit <= 2^126, so the candidate fits in u128. + let candidate = res.checked_add(bit).expect("isqrt candidate overflow"); + if num >= candidate { + num = num.checked_sub(candidate).expect("isqrt remainder underflow"); + res = (res >> 1).checked_add(bit).expect("isqrt result overflow"); + } else { + res >>= 1; + } + bit >>= 2; + } + res +} + +/// Integer square root for signed 128-bit values. +/// +/// Returns `floor(sqrt(n))`, or `0` when `n <= 0`. The result of `i128::MAX` +/// is about `1.3e19` and always fits in `i128`. +pub fn isqrt_i128(n: i128) -> i128 { + if n <= 0 { + return 0; + } + // n > 0 here, so the cast to u128 is lossless. + let root = isqrt_u128(n as u128); + // root <= 2^63 for any positive i128, so the cast back is lossless. + root as i128 +} + +/// Checked quadratic cost of moving from `prev_votes` to `prev_votes + additional`. +/// +/// Computes `(prev + additional)^2 - prev^2` (the incremental cost of quadratic +/// voting, where a total of `V` votes costs `V^2` credits). Returns `None` if any +/// intermediate step overflows `i128`. +pub fn quadratic_cost(prev_votes: u64, additional: u64) -> Option { + let prev = i128::from(prev_votes); + let total = prev.checked_add(i128::from(additional))?; + let total_sq = total.checked_mul(total)?; + let prev_sq = prev.checked_mul(prev)?; + total_sq.checked_sub(prev_sq) +} diff --git a/contracts/quadratic_voting/src/test.rs b/contracts/quadratic_voting/src/test.rs index 5fe1fc1d3..b5a27d81e 100644 --- a/contracts/quadratic_voting/src/test.rs +++ b/contracts/quadratic_voting/src/test.rs @@ -193,3 +193,71 @@ fn property_terminal_vote_state_is_consistent() { client.vote(&user, &1, &7); assert_eq!(client.get_user_credits(&user), 100 - 49); } + +#[test] +fn isqrt_boundaries() { + assert_eq!(math::isqrt_u128(0), 0); + assert_eq!(math::isqrt_u128(1), 1); + assert_eq!(math::isqrt_u128(3), 1); + assert_eq!(math::isqrt_u128(4), 2); + assert_eq!(math::isqrt_u128(99), 9); + assert_eq!(math::isqrt_u128(100), 10); + assert_eq!(math::isqrt_u128(u128::MAX), u64::MAX as u128); + + assert_eq!(math::isqrt_i128(0), 0); + assert_eq!(math::isqrt_i128(1), 1); + assert_eq!(math::isqrt_i128(3), 1); + assert_eq!(math::isqrt_i128(4), 2); + assert_eq!(math::isqrt_i128(99), 9); + assert_eq!(math::isqrt_i128(100), 10); + assert_eq!(math::isqrt_i128(-1), 0); + assert_eq!(math::isqrt_i128(i128::MIN), 0); +} + +#[test] +fn isqrt_i128_max_does_not_panic_and_is_floor() { + let r = math::isqrt_i128(i128::MAX); + // r is floor(sqrt(MAX)): r^2 <= MAX < (r+1)^2. + assert!(r.checked_mul(r).is_some()); + assert!(r.checked_mul(r).unwrap() <= i128::MAX); + assert!((r + 1).checked_mul(r + 1).is_none()); +} + +#[test] +fn isqrt_matches_exact_for_perfect_squares() { + for k in 0u128..=2_000 { + assert_eq!(math::isqrt_u128(k * k), k); + if k > 0 { + assert_eq!(math::isqrt_u128(k * k - 1), k - 1); + } + } +} + +#[test] +fn quadratic_cost_values() { + assert_eq!(math::quadratic_cost(0, 0), Some(0)); + assert_eq!(math::quadratic_cost(0, 3), Some(9)); + assert_eq!(math::quadratic_cost(3, 2), Some(16)); // 25 - 9 + assert_eq!(math::quadratic_cost(3, 4), Some(40)); // 49 - 9 + assert_eq!(math::quadratic_cost(u32::MAX as u64, 1), Some( + (u32::MAX as i128 + 1).pow(2) - (u32::MAX as i128).pow(2) + )); +} + +#[test] +fn quadratic_cost_overflow_returns_none() { + assert_eq!(math::quadratic_cost(u64::MAX, u64::MAX), None); +} + +#[test] +fn vote_incremental_cost_is_quadratic() { + let env = Env::default(); + env.mock_all_auths(); + let (client, _) = setup(&env); + let user = Address::generate(&env); + client.create_proposal(&user, &String::from_str(&env, "p")); + client.vote(&user, &1, &3); // cost 9 + client.vote(&user, &1, &2); // total 5 -> cost 25, incremental 16 + assert_eq!(client.get_user_credits(&user), 100 - 25); + assert_eq!(client.get_proposal(&1).votes_received, 5); +}