Skip to content

docs: improved sidebar and usage in DAPI client#3

Merged
shumkov merged 1 commit into
v0.22-devfrom
sidebar-and-usage
Nov 3, 2021
Merged

docs: improved sidebar and usage in DAPI client#3
shumkov merged 1 commit into
v0.22-devfrom
sidebar-and-usage

Conversation

@shumkov
Copy link
Copy Markdown
Collaborator

@shumkov shumkov commented Nov 3, 2021

Issue being fixed or feature implemented

The DAPI-Client documentation missed some loves and was let outdated with the recent additions/removal of methods and the migration towards .core and .platform.
This PR fixes it.

What was done?

  • added examples html file
  • documentation set up-to-date

How Has This Been Tested?

N/A

Breaking Changes

N/A

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone
@shumkov shumkov changed the base branch from master to v0.22-dev November 3, 2021 14:38
@shumkov shumkov added this to the v0.22.0 milestone Nov 3, 2021
@shumkov shumkov merged commit 30d195d into v0.22-dev Nov 3, 2021
@shumkov shumkov deleted the sidebar-and-usage branch November 3, 2021 14:40
thephez referenced this pull request in thephez/platform Nov 29, 2021
Co-authored-by: Alex Werner <Alex-Werner@users.noreply.github.com>
shumkov pushed a commit that referenced this pull request Nov 23, 2022
shumkov pushed a commit that referenced this pull request Nov 23, 2022
QuantumExplorer added a commit that referenced this pull request Mar 16, 2026
- Delete fake tests in make_sure_core_is_synced_to_chain_lock/v0 that
  never called the production method (CRITICAL #1)
- Delete fake tests in verify_chain_lock_through_core/v0 that only
  exercised mock wiring, never the production method (CRITICAL #2)
- Add real tree verification in protocol_upgrade tests: v12 now checks
  shielded pool, notes, nullifiers, and anchors trees (MAJOR #3)
- Fix get_contract_with_fetch_info_and_fee assertion to verify the
  contract is Some, not just Ok (MEDIUM #4)
- Rename test_transition_from_version_10 and add actual verification
  of v11/v12 artifacts (LOW #5)
- Replace bare .unwrap() with .expect() in choose_quorum tests (LOW #6)
- Remove empty-input smoke test for store_address_balances (LOW #7)
- Fix test_decode_state_transition_at_exact_max_size to assert the
  correct property (not rejected as oversized) rather than incorrectly
  assuming zeros cannot decode

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
QuantumExplorer added a commit that referenced this pull request Apr 19, 2026
Collapse the (wallet, account, pool) grouping into (wallet, account).
Each account gets a single section; External / Internal / Absent pool
entries all appear under the same header, sorted by (poolTag,
addressIndex). The per-address row now leads with the pool name
("External · #3 · used") so pool identity is still obvious without
inflating the section list.

Fixes the prior visual confusion where a single BIP44 account
produced two adjacent sections (External + Internal) and the sticky
header didn't keep up with the currently-visible pool.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
shumkov added a commit that referenced this pull request Apr 29, 2026
…rs DPP)

Reverses an earlier wrong call. Initial review of thepastaclaw's #3 finding
concluded "DPP doesn't enforce a specific encrypted_note size, so wasm
shouldn't either" — a comprehensive cross-layer audit shows that's wrong:

- DPP: `pub const ENCRYPTED_NOTE_SIZE: usize = 216;` in
  `state_transitions/shielded/common_validation.rs:12`
- DPP enforces `action.encrypted_note.len() != ENCRYPTED_NOTE_SIZE` →
  `ShieldedEncryptedNoteSizeMismatchError` (line 65), called from all 5
  shielded transition validators.
- Drive-ABCI imports the same const + adds a `const _: ()` compile-time
  assertion that 32 + 104 + 80 = 216 (`shielded_common/mod.rs:55-58`) and
  re-validates at runtime (line 95).
- The grovedb Orchard library's `TransmittedNoteCiphertext` exposes
  fixed-size slices (`[u8; 32]`, `[u8; 104]`, `[u8; 80]`) — the layout is
  hardware-locked.

The developer comment in `shielded/builder/mod.rs:358` showing uncertainty
("wait — 84+512?") is outdated. The canonical size is 216 bytes, enforced
at multiple layers.

Mirror that constraint at the wasm boundary by switching `try_to_bytes` to
`try_to_fixed_bytes::<ENCRYPTED_NOTE_SIZE>` in `SerializedOrchardActionWasm`'s
constructor. Reuses the DPP constant directly so the size never drifts. Now
JS callers get an immediate, clear "encryptedNote must be exactly 216 bytes"
at construction instead of a `ShieldedEncryptedNoteSizeMismatchError` later
during DPP validation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
QuantumExplorer added a commit that referenced this pull request May 7, 2026
… drop lock before persister I/O

Addresses three findings from the @thepastaclaw review on commit
757f035 (the FFI/Swift wiring + tx-bytes plumbing in d8de2c2 and
8e9b75d already addressed the fourth — "FFI not wired" — and
already reach the iOS build through the new
`on_get_core_tx_record_fn` callback).

## #2 — fast-fail regression in `upgrade_to_chain_lock_proof`

Pre-PR, a missing in-memory record errored fast with
`AssetLockProofWait("Transaction X not found in account Y")`.
After the helper landed, the missing case fell through to
`wait_for_chain_lock(...)` and burned the full chainlock timeout
before returning `FinalityTimeout` — a regression for genuine
"tx never tracked" cases (wallet-state mismatch, post-wipe race).

Restored the fast-fail: when both the in-memory map and the
persister miss, the function returns the same
`AssetLockProofWait` error pre-PR callers expected. The
"persister returned a non-chainlocked record" case still falls
through to `wait_for_chain_lock` (correct — the tx exists, just
hasn't chainlocked yet).

## #3 — silent error downgrade on transient persister failure

`record_or_persister` previously logged backend errors at
`debug` and surfaced them as `None`. That's correct for the
poll loops (`wait_for_chain_lock`, `wait_for_proof`) where the
next tick retries — but wrong for `resolve_status_from_info`,
which is a one-shot recovery call: a transient backend error
silently classified the lock as `AssetLockStatus::Broadcast`
even when the underlying tx was actually chain-locked,
forcing `resume_asset_lock` down the wasteful
`Broadcast → wait_for_proof` path with no operator-visible
signal.

Split the helper:

* `record_or_persister` now returns
  `Result<Option<TransactionRecord>, PersistenceError>`. Call
  sites choose their own policy.
* `record_or_persister_or_log` is the new poll-loop variant —
  same `Option` return, but logs persister errors at `warn` (up
  from `debug`) before downgrading to `None`. A persistent
  failure is now visible in operator logs.
* The two poll-loop sites (`wait_for_chain_lock`,
  `wait_for_proof`) switched to `_or_log`.
* The two fast-fail sites (`validate_or_upgrade_proof`,
  `upgrade_to_chain_lock_proof`) propagate `Err` as
  `AssetLockProofWait("Persister lookup for tx X failed: …")`,
  distinct from the "neither lookup found it" message.
* The one-shot recovery site (renamed
  `resolve_status_with_in_memory`, see #4 below) logs at
  `error` and degrades to `None`, preserving recovery's "always
  make some progress" semantics while making the failure
  visible.

Tests updated for the new signature. Added a new test
`record_or_persister_or_log_swallows_backend_errors_as_none`
covering the poll-loop variant; renamed the old "swallows" test
to `propagates_backend_errors` and asserts `Err`. 121/121 tests
pass.

## #4 — wallet-manager lock held across persister I/O

`recover_asset_lock_blocking` was acquiring
`self.wallet_manager.blocking_write()` and then dispatching to
`resolve_status_from_info`, which after the helper's persister
fallback could perform synchronous SQLite/SwiftData/FFI I/O
behind the write lock — serializing other writers on a single
disk lookup.

Restructured into three phases:

1. **Lock held**: claim the `tracked_asset_locks` slot,
   snapshot the in-memory record (only when no proof was
   provided), drop the lock.
2. **No lock held**: resolve status, including the persister
   fallback. The renamed
   `resolve_status_with_in_memory(snapshot, ...)` takes the
   pre-snapshotted record so this phase no longer touches the
   wallet manager.
3. **Lock held**: re-acquire to commit the
   `TrackedAssetLock` entry. A re-check of
   `tracked_asset_locks.contains_key` handles the case where
   another caller raced in during phase 2 (first writer wins).

Net effect: the persister I/O now runs without serializing
other wallet-manager writers. The lock-hold time drops from
"in-memory lookup + persister round-trip" to two short
in-memory operations.

Verification:
- `cargo fmt --all`: clean
- `cargo test -p platform-wallet --lib`: 121/121 (was 120,
  +1 from the new `_or_log` test)
- `bash packages/swift-sdk/build_ios.sh --target sim --profile dev`:
  BUILD SUCCEEDED end-to-end including SwiftExampleApp link

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
QuantumExplorer added a commit that referenced this pull request May 11, 2026
…logists

Sweeps the new comments introduced in this PR for narrative phrases
that age poorly — "PR #3623 review comment r3214794852", "Codex review
finding #3", "grovedb#656/#658", "what used to be ~30-line per-mode
match arms", "the prior merge-based code", "earlier drafts of the v12
endpoint", "pre-refactor behavior", etc. — and rewrites them to
explain what the code does and why, without requiring the reader to
have lived through the PR's history.

Specific rewrites:

- `rs-drive-abci/src/query/document_count_query/v0/mod.rs`:
  - `test_documents_count_with_in_and_prove_returns_proof` docstring
    drops the "PR #3623 review comment" + commit-hash narrative and
    states the two end-to-end guarantees the test pins (routing →
    PointLookupProof; dispatcher → threads order_by).
  - `test_documents_count_range_with_prove_and_distinct_returns_proof`
    drops the "earlier commits rejected this" + commit-hash phrasing
    and describes the dispatch path + wire-shape contract directly.

- `rs-drive/src/query/drive_document_count_query/`:
  - `drive_dispatcher.rs`: module + impl docstrings drop "collapse
    what used to be ~30-line per-mode match arms" framing; describe
    the per-mode contract on its own terms.
  - `execute_point_lookup.rs`: "Codex review finding #3" → describes
    the dedupe invariant directly.
  - `execute_range_count.rs`: "cursor field used to exist but was
    removed before v12 shipped" → "no cursor field because single-
    `bytes` would be ambiguous for compound queries"; "the prior
    merge-based code" → "bounded by the contract author's index
    choice".
  - `tests.rs`: drops "Codex review finding #3" and "pre-refactor
    behavior" framings.

- `rs-drive/src/drive/contract/insert/insert_contract/v0/mod.rs`:
  - `range_count_with_in_on_prefix_returns_per_brand_color_entries`:
    "cross-fork merging was dropped (originally bg of Codex…)" →
    explains why merging pre-limit can undercount.
  - `range_count_executor_accepts_starts_with_in_all_four_modes`:
    drops "earlier commits rejected `StartsWith`, this is the
    rewrite".
  - `aggregate_count_proof_verifies_on_compound_index_with_equal_prefix`:
    "grovedb#658's multi-layer envelope" → "grovedb's multi-layer
    aggregate-count proof envelope".
  - `aggregate_count_proof_counts_cars_in_parking_lots_greater_than_b`:
    "Real-world scenario test for grovedb#656's primitive" → "Scale
    test for the AggregateCountOnRange proof primitive".
  - `range_count_executor_returns_per_lot_counts_…`: drops "no-proof
    companion to grovedb#656's primitive" framing.
  - `distinct_count_proof_with_in_on_prefix_returns_…`: "this was the
    original motivation for the no-merge design, Codex finding 1" →
    direct explanation of why server-side merging would undercount
    under per-fork limits.

- `rs-sdk/src/platform/documents/document_count_query.rs`:
  - struct docstring drops "added in PR #3623" framing.
  - `limit` field's pagination note drops the "cursor field existed
    earlier but was removed before v12 shipped" narrative.

- `wasm-sdk/src/queries/document.rs`:
  - `getDocumentsCount` docstring drops "unified successor to the
    previous getDocumentsCount / getDocumentsSplitCount pair"
    framing.

- `book/src/drive/document-count-trees.md`:
  - Pagination paragraph drops "existed in earlier drafts of the v12
    endpoint but was removed before shipping" narrative.

No behavior change, no public-API surface change. Verified clippy +
fmt clean across the touched crates with `-D warnings`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
shumkov added a commit that referenced this pull request May 13, 2026
Unit tests pin the filter / predicate / round-trip invariants;
runtime composition (SwiftData @query reactivity, coordinator
@published mutations, view re-renders, SPV event routing) needs
manual testnet validation. Six scenarios cover the happy path, the
🔴 double-tap-during-in-flight guard, crash recovery from both
pre-IS-lock (status 1) and post-IS-lock (status 2/3) states, the
failed-retry flow, and the `.completed` retention window.

Also documents which upstream PR #3549 issues are tangential to our
UAT (#1 / #5: different code paths; #2: mitigated by the
persister's proofBytes capture at the IS-lock arrival moment; #3 /
#4: doc fixes only).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
QuantumExplorer added a commit that referenced this pull request May 14, 2026
…iagrams

New chapter under "Drive" that walks through the bench fixture's
widget contract and shows what each count-query proof actually
proves — both the path query the prover signs AND the verified
element the verifier extracts. Every example is reproducible
against the existing bench at
`packages/rs-drive/benches/document_count_worst_case.rs`.

Seven worked examples (the `group_by = []` proof surface):

1. `(empty)` — primary-key CountTree fast path, 585 B.
2. `brand == X` — PointLookup over byBrand (plain countable),
   1,041 B post-v12 (the value-tree-direct shape now activates
   for any countable terminator, not just rangeCountable).
3. `color == X` — PointLookup over byColor (rangeCountable),
   1,327 B. Same shape as #2 — the rangeCountable flag is only
   relevant for #7.
4. `brand == X AND color == Y` — PointLookup over byBrandColor;
   the proof descends through the byBrand value tree's
   `NonCounted`-wrapped `color` continuation to the byBrandColor
   terminator, 1,911 B.
5. `brand IN [b0, b1]` — outer Keys per In value, no subquery
   (the value-tree-direct shape on the In axis), 1,102 B.
6. `color IN [c0, c1]` — identical shape to #5, surfacing the
   v12 generalization at a glance: byBrand (plain countable)
   and byColor (rangeCountable) produce structurally identical
   proofs, 1,381 B.
7. `color > floor` — AggregateCountOnRange over byColor's
   ProvableCountTree; different verifier
   (`verify_aggregate_count_query`), single u64 result, 2,072 B.

Each section has three parts:

- **Path query** (decoded path + items + subquery), the prover's
  spec.
- **Verified element / payload** (the structured output of
  `verify_query`/`verify_aggregate_count_query`).
- **Mermaid diagram** with:
  - The tree element wrapper for the relevant GroveDB subtree.
  - Blue arrows tracing the descent.
  - A cyan target node for the verified element.
  - Faded nodes for context.

A "GroveDB Layout" diagram up front shows the storage shape for
the whole contract (doctype CountTree primary key, byBrand
NormalTree property-name with CountTree value trees and
NonCounted-wrapped byBrandColor continuations, byColor
ProvableCountTree property-name with CountTree value trees).
Each per-query diagram is a focused slice of this overview.

An at-a-glance comparison table at the end summarizes primitive
choice, verified shape, and proof size across all seven examples
to make the structural symmetry between #2/#3 and #5/#6 (post-v12
generalization) and the asymmetry of #7 (AggregateCountOnRange)
visually obvious.

Registered under `Drive` in `SUMMARY.md` immediately after the
existing `Document Count Trees` chapter, which it complements
(that chapter explains the tree variants in the abstract; this
one shows how queries traverse the resulting layout). Builds
cleanly via `mdbook build` against the existing
`mdbook-mermaid` preprocessor configuration.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
QuantumExplorer added a commit that referenced this pull request May 18, 2026
Addresses Codex findings #2 (partial), #3 (full), #5 (full):

## #3 Sum aggregation overflow (P1) — full fix

The proof and no-proof paths used inconsistent overflow semantics that
were both wrong for a deterministic protocol:
- `execute_range_sum.rs:113` used `saturating_add` (silent clamp)
- `execute_point_lookup.rs:48` used iterator `.sum::<i64>()` (panics
  in debug, wraps in release)

Replaced both with `checked_add` + a typed `QuerySyntaxError::Unsupported`
error so an overflowed aggregate fails deterministically with a clear
user-facing message at the same point in both paths.

## #5 Stale gRPC clients (P2) — full fix

`yarn build` in `packages/dapi-grpc/` regenerates the static clients
for web, Node.js, Objective-C, and Python (Java is service-stub only).
All five now include `SumResults`, `AverageResults`, `sums`,
`averages`, `aggregate_sum`, `aggregate_average`, and the entry types.
The Rust client uses `tonic_prost_build` at compile time so it was
never stale.

## #2 U64 summable values (P1) — partial fix

The deeper issue (DPP accepts U64 summable; values > i64::MAX would
silently overflow grovedb's i64 sum aggregator) needs either schema-
inference restructuring or a document-level validator — neither
small enough for this PR. The symptom is mitigated here:

`read_document_sum_contribution` now returns
`DriveError::InvalidInput` (user-facing) instead of
`CorruptedCodeExecution` (internal corruption signal) on i64
conversion failure, with the property name and the underlying
conversion error interpolated into the message. A user submitting a
document whose sum-bearing property value exceeds i64::MAX now gets
a clean "value cannot be represented as i64" error rather than an
internal corruption error. The DPP-level rejection of U64 is tagged
as a TODO with the design context preserved.

drive 3165/3165, dpp 3461/3467 (6 ignored). clippy
--workspace --all-features -D warnings clean.

## Skipped (tracked follow-ups)

- #1 cost estimation paths still use `Element::required_item_space` —
  needs grovedb's `required_item_with_sum_item_space` helper which
  doesn't exist upstream yet.
- #4 mixed count/sum shared-prefix indexes — needs a failing repro
  test to demonstrate the path is reachable from valid contracts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant