Skip to content

financing_pool.mark_default's best-effort risk_registry.try_record_default() call silently swallows failures with no event or observability path #521

Description

@OxDev-max

Description

contracts/financing_pool/src/lib.rs:632-643:

// Automatically record the default against the SME in the risk registry
let invoice = nft_client.get_invoice(&invoice_id);
if let Some(rr_contract) = env.storage().instance().get::<DataKey, Address>(&DataKey::RiskRegistry) {
    let rr_client = kora_risk_registry::RiskRegistryContractClient::new(&env, &rr_contract);
    // Best-effort: ignore errors if SME is not registered in risk registry
    let _ = rr_client.try_record_default(&admin, &invoice.sme);
}

docs/ARCHITECTURE.md's cross-contract call table (line 255) documents this as
financing_pool -> risk_registry: record_default() (best-effort, may fail silently), confirming
it's a deliberate design choice, not an oversight in the call itself. However, the failure is
let _ =-discarded with no event emitted, no counter incremented, no log of any kind — if
try_record_default fails (e.g. the SME was never registered, the SME's profile was concurrently
modified, or risk_registry itself is unreachable/misconfigured), there is currently no way for
an operator or monitoring system to ever discover that a real default occurred without the
corresponding risk-registry bookkeeping being updated.

Requirements and Context

Given risk_registry's SME defaults counter is the primary signal several other features
(tier-based fees, future underwriting decisions) depend on, an unnoticed silent drift between
"invoices actually defaulted" and "risk_registry's recorded default count" is a real operational
risk. This is a genuine cross-contract error-handling/observability gap distinct from the
existing closed reconciliation-view issues (#200, #224), which focus on balance/state
consistency rather than this specific silent-failure code path.

Suggested Execution

  1. git checkout -b feature/mark-default-best-effort-observability
  2. Add a new event (in kora_shared::events, respecting issue Refactor Invoice NFT Contracts #1's symbol-length constraint)
    emitted specifically when try_record_default fails, including the invoice_id and sme
    address, so off-chain monitoring can alert on it.
  3. Consider whether mark_default should also update a durable on-chain counter (e.g. in
    ProtocolStats) tracking "risk-registry-sync failures," giving scripts/health-check.sh a
    concrete metric to surface.
  4. Add an integration test that forces try_record_default to fail (e.g. an SME never
    registered in risk_registry) and asserts the new failure event/counter fires, while
    mark_default itself still succeeds (preserving the intentional best-effort semantics for
    the primary default-marking flow).
  5. Extend scripts/health-check.sh and/or scripts/check_state_drift.sh to surface this new
    signal.

Acceptance Criteria

  • A distinct event (or durable counter) is emitted whenever the best-effort risk_registry.try_record_default call fails inside mark_default
  • mark_default's primary success path is unaffected — it still succeeds even when the risk-registry sync fails, preserving documented behavior
  • A test forces the failure path and asserts the new observability signal fires
  • docs/ARCHITECTURE.md's cross-contract call table is updated to describe the new failure-visibility mechanism

Guidelines: PR description must include Closes #<issue-number>.
Complexity: High (200 points)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions