Skip to content

chore: synchronize packages dependency versions#1

Merged
shumkov merged 14 commits into
masterfrom
feat-prepare-for-work
Nov 4, 2021
Merged

chore: synchronize packages dependency versions#1
shumkov merged 14 commits into
masterfrom
feat-prepare-for-work

Conversation

@antouhou
Copy link
Copy Markdown
Contributor

@antouhou antouhou commented Nov 3, 2021

Issue being fixed or feature implemented

The build doesn't work because different packages use different versions of dependencies

What was done?

Synchronized dependency versions across all packages

How Has This Been Tested?

Run the build and run dashmate

Breaking Changes

None

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 title feat: prepare repository for work chore: prepare repository for work Nov 3, 2021
Konstantin Shuplenkov added 2 commits November 3, 2021 18:13
# Conflicts:
#	packages/js-dash-sdk/package-lock.json
#	packages/js-dash-sdk/package.json
#	packages/js-dpp/package-lock.json
#	packages/js-dpp/package.json
@shumkov shumkov changed the title chore: prepare repository for work chore: prepare packages Nov 3, 2021
@antouhou antouhou changed the title chore: prepare packages chore: synchronize packages versions Nov 4, 2021
@antouhou antouhou changed the title chore: synchronize packages versions chore: synchronize packages dependency versions Nov 4, 2021
@antouhou antouhou marked this pull request as ready for review November 4, 2021 10:49
Copy link
Copy Markdown
Collaborator

@shumkov shumkov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍

@shumkov shumkov merged commit 9097962 into master Nov 4, 2021
@shumkov shumkov deleted the feat-prepare-for-work branch November 4, 2021 12:42
jawid-h added a commit that referenced this pull request Dec 27, 2021
jawid-h added a commit that referenced this pull request Dec 27, 2021
@shumkov shumkov added this to the v0.21.x milestone Feb 1, 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>
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
…0) uniformity, FFI limit decode

Three independent review findings from thepastaclaw / coderabbitai
addressed together because the proto contract change in #2 is the
single source of truth for both server (#2) and client (#4).

**#1: Unknown `Select` enum discriminant** classified as
`QueryError::InvalidArgument` instead of `not_yet_implemented`.
`Select::try_from(42)` is structurally malformed wire input — there's
no future protocol value that would make `42` a valid `Select`
behavior, so the future-capability error class (with its "valid
request structure, callers can keep it unchanged when capability
lands" contract from `not_yet_implemented`'s docstring) is the
wrong class. New test `reject_unknown_select_enum_value_as_invalid_argument`
pins the discriminator so a future refactor that re-collapses the
two error classes for "consistency" fails loudly.

**#2: `limit: Some(0)` uniformly invalid across SELECT modes.**
Pre-fix, three legacy behaviors collided on the same wire value:
- `SELECT DOCUMENTS` `unwrap_or(0)` and forwarded to v0, where
  `limit=0` is "use server default" (accept-as-default).
- `SELECT COUNT, {Aggregate, GroupByIn}` rejected via `is_some()`
  with mode-specific message (reject-as-invalid).
- `SELECT COUNT, {GroupByRange, GroupByCompound}` passed `Some(0)`
  to drive (accept-as-zero).

Three semantics for the same wire bytes is bad contract. The v1
wire's whole point of switching to `optional uint32` was to make
"unset" explicit (`None`), so `Some(0)` only makes sense as an
*explicit* zero — structurally meaningless regardless of mode.
Centralized `limit == Some(0)` rejection at the top of
`validate_and_route`; the existing per-mode `is_some()` checks
still catch `Some(N>0)` correctly. Updated the documents-path
`unwrap_or(0)` comment to note `Some(0)` can't reach it. Proto
docstring on `optional uint32 limit` calls out the cross-mode
contract explicitly. New test
`reject_limit_some_zero_uniformly_across_select_modes` exercises
all 5 mode combinations and asserts the centralized message fires.

**#4: FFI `dash_sdk_document_count` limit decode matches docs.**
Pre-fix doc said `-1` = unset, `≥0` = explicit cap. Implementation
mapped every negative value to `0` (SDK's unset sentinel), and `0`
was also the SDK's unset sentinel — so `-1`, `-2`, `-100`, AND `0`
all silently meant "use server default", masking caller bugs from
uninitialized memory / arithmetic underflow / etc.

Extracted the decode into `decode_ffi_limit(i64) -> Result<u32, _>`
so it's unit-testable in isolation. New contract is single-valued
per input: `-1` → unset sentinel; `> 0` → explicit cap; `0` and
`< -1` → rejected at the FFI boundary with messages directing
callers to valid alternatives. Server-side #2 rejection happens
anyway, but surfacing the rejection at the FFI is faster and
mode-independent. 5 new tests cover each sentinel category
(minus_one_is_unset, zero_is_rejected, negative_other_than_minus_one_is_rejected,
positive_decodes_verbatim, over_u32_max_is_rejected).

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.
QuantumExplorer added a commit that referenced this pull request May 18, 2026
…ue in cost estimation

Bumps grovedb pin to develop head `e47626e96288b0052614cffd0451cefcf4b1103d`
which lands grovedb#673 (`required_item_with_sum_item_space` +
`required_reference_with_sum_item_space` helpers), then wires the new
helpers at the per-element stateless-cost / dry-run estimation sites
this PR added.

## Sites wired

All three sites are v12+ gated (no v11 consensus baseline), so the
switch from `required_item_space` → sum-aware helper is unconditional:

- `add_document_to_primary_storage/v0/mod.rs:591` —
  `DocumentEstimatedAverageSize` arm now picks
  `required_item_with_sum_item_space` when
  `primary_key_sum_property.is_some()`. Also adds +10 to the
  `StatelessBatchInsert` target size for the same predicate so the
  full insert cost (element + target) reflects the i64 sum bytes.
- `add_reference_for_index_level_for_contract_operations/v0/mod.rs:236`
  (non-unique) and `:280` (unique) — both `DocumentEstimatedAverageSize`
  arms now pick `required_reference_with_sum_item_space` when
  `sum_property_name.is_some()`.

## Tracked follow-ups (need either grovedb API extensions or version-gated workarounds)

Codex finding #1 also called out two layer-level estimation sites
that can't be cleanly fixed with the current grovedb API:

- `address_funds/estimated_costs/for_address_balance_update/v0/mod.rs:92`
  uses `EstimatedLayerSizes::AllItems(key, value, flags)` — grovedb's
  enum has no `AllItemsWithSumItem` variant. The only workaround is
  bumping `AVERAGE_NONCE_SIZE + AVERAGE_BALANCE_SIZE` from 14 to 24
  via v0/v1 dispatch (consensus-relevant; v11 grandfathered).
- `add_estimation_costs_for_contract_insertion/v1/mod.rs:154` reports
  `sum_trees_weight: 0` / `count_sum_trees_weight: 0` because the
  surrounding loop only counts `range_countable` terminators —
  doesn't visit `range_summable` / `range_countable + range_summable`
  shapes.

Both are tracked for a separate PR once we decide between extending
grovedb's `EstimatedLayerSizes` enum or shipping the constant-bump
workaround.

drive 3165/3165, dpp 3461/3467 (6 ignored). clippy
--workspace --all-features -D warnings clean.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants