Parsers silently produce empty strings for missing/non-string required fields

[gyorgybalazsi]

Follow-up from Copilot review of #46.

Context

Every flat-event parser extracts required fields with .as_str().unwrap_or(""). If the server response contains a matching event envelope but is missing or has non-string values for contractId, templateId, createdEventBlob, or fields inside createArgument, the parsers silently produce empty strings and continue. Downstream callers then fail with misleading errors — e.g., a missing contractId from create_deposit_account produces:

Created DepositAccount  not found in active contracts

…instead of a clear parse-time error like \contractId` missing or not a string`.

This pattern is pre-existing — the tree-shape parsers used the same unwrap_or("") shape, and #41 / #46 carried it forward unchanged. It is not a regression from either of those PRs, but it is a real shipping-confidence issue.

Copilot flagged it on PR #46:

• fix: v0.4.1 parser regression — migrate cbtc-lib parsers to typed response models #46 (comment) (mint)
• fix: v0.4.1 parser regression — migrate cbtc-lib parsers to typed response models #46 (comment) (redeem ×2, the second instance at submit_withdraw line ~165 is the suppressed-low-confidence comment)
• fix: v0.4.1 parser regression — migrate cbtc-lib parsers to typed response models #46 (comment) (credentials)

Affected sites (all 7 parsers)

• src/mint_redeem/mint.rs:114 parse_created_deposit_account_cid — contractId
• src/mint_redeem/redeem.rs:138 parse_created_withdraw_account_cid — contractId
• src/mint_redeem/redeem.rs:165 parse_submit_withdraw_response — contractId, plus createdEventBlob and createArgument go into JsActiveContract without validation
• src/transfer.rs:607 parse_transfer_response_value — senderChangeCids filter-maps via as_str() (drops non-strings silently); output.value.transferInstructionCid uses as_str() early-return is OK but doesn't distinguish missing-vs-not-yet
• src/consolidate.rs parse_consolidate_response — extracts receiverHoldingCids with similar lenient pattern
• src/split.rs parse_split_response — output_cid returns from as_str().ok_or(...) (already strict); senderChangeCids lenient
• src/credentials.rs:448 parse_accept_credential_offer_response — contractId, createdEventBlob, createArgument

The strictness varies site-by-site today. A consistent treatment would be helpful.

Proposal

Replace as_str().unwrap_or("") with as_str().ok_or_else(|| format!(\"{} missing or not a string in <Event>\", field)) for every required field across all 7 parsers. Specifically:

• contractId is always required → strict.
• templateId is used in suffix-match — empty matches nothing, so the existing pattern accidentally produces correct behavior. Still strict is clearer.
• createdEventBlob — sometimes empty in practice (Canton omits it when irrelevant), so it should remain unwrap_or(\"\"). Document inline.
• createArgument is required for the from_active_contract calls in submit_withdraw and accept_credential_offer.

Plus: extend the fixture tests in each mod parser_tests block with one more case: malformed_event_missing_required_field_returns_err — fixture has the matching template but a missing contractId, parser should Err, not Ok(\"\").

Risk

Small behavioral change: callers that previously received a misleading downstream error now receive a clearer parse-time error. No code path that currently returns Ok would start returning Err for a previously-working response — only malformed responses change behavior.

Out of scope

• Restructuring parsers further.
• Changing public API.
• Touching the integration tests (those are separate concerns tracked in integration_test.rs Step 20 (split) picks an invalid holding when pending-withdraw exists #48, integration_test.rs Step 19 (consolidate) threshold too high to exercise consolidate parser #49).

[gyorgybalazsi]

Update: PR #46 commit 6247d31 (Layer 3) largely obviates this issue at the type level. Parsers now deserialize into JsSubmitAndWaitForTransactionResponse rather than walking raw serde_json::Value, so missing or non-string required fields (contractId, templateId, createdEventBlob) become Serde deserialization errors at parse time instead of Ok("") followed by confusing downstream failures.

Remaining scope (if any) is the inner Daml-encoded value walks — exerciseResult["output"]["value"]["receiverHoldingCids"] and similar — where the Daml choice return type stays Value because it's not part of the Ledger API schema. Those still use lenient access patterns, but they're a much narrower target.

Pending PR #46 review: closing this issue if the typed refactor lands; reopening narrowed-scope if any remaining inner-walk strictness concerns emerge.