Skip to content

fix: possible overflow issues#877

Merged
shumkov merged 6 commits into
v0.24-devfrom
fix/overflows
Apr 4, 2023
Merged

fix: possible overflow issues#877
shumkov merged 6 commits into
v0.24-devfrom
fix/overflows

Conversation

@qrayven

@qrayven qrayven commented Mar 31, 2023

Copy link
Copy Markdown
Contributor

Issue being fixed or feature implemented

What was done?

The PR fixes potential overflow panics for artithmetic operations

How Has This Been Tested?

Breaking Changes

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

@qrayven
qrayven marked this pull request as ready for review March 31, 2023 10:39
@shumkov shumkov changed the title Fix overflow issues fix: possible overflow issues Apr 4, 2023
shumkov
shumkov previously approved these changes Apr 4, 2023

@shumkov shumkov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍

shumkov added 3 commits April 4, 2023 17:45
# Conflicts:
#	packages/rs-dpp/src/identity/state_transition/identity_update_transition/validate_identity_update_transition_state.rs

@shumkov shumkov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍

@shumkov
shumkov merged commit 32ddec7 into v0.24-dev Apr 4, 2023
@shumkov
shumkov deleted the fix/overflows branch April 4, 2023 14:12
markin-io pushed a commit that referenced this pull request Apr 26, 2023
Co-authored-by: Ivan Shumkov <ivan@shumkov.ru>
@markin-io markin-io mentioned this pull request Apr 26, 2023
5 tasks
markin-io pushed a commit that referenced this pull request Apr 26, 2023
Co-authored-by: Ivan Shumkov <ivan@shumkov.ru>
bfoss765 added a commit to bfoss765/platform that referenced this pull request Jul 14, 2026
…edes local dashpay#873 vendor)

Testing dashpay#866+dashpay#851 at gap 30 pending on-device verdict.

Swaps OUR dashpay#846 committed-range fix for hash's alternative
(rust-dashcore PR dashpay#866, `rescan_committed_range`: newly derived scripts
are re-tested against the persisted committed filter range below the
committing batch), adds PR dashpay#851 (the dashpay#649 out-of-order
spend-before-funding UTXO fix), and forces DEFAULT_COINJOIN_GAP_LIMIT to
30 in the vendored third_party/rust-dashcore — to test on-device whether
hash's fixes make the narrower gap viable on the heaviest CoinJoin
wallet, instead of the dashj-parity widening to 100 (dashpay#868).

Vendored tree transform (third_party/rust-dashcore):
  - REMOVED our dashpay#846 machinery (drain_committed_rescans /
    committed_filters retention across the commit boundary).
  - APPLIED PR dashpay#866 (2 commits: 8ccba0b6 rescan committed ranges +
    381a4fec defer backward sweep to forward quiescence). Ships its own
    coinjoin_gap_discovery_tests.rs — the committed-batch repro
    (coinjoin_gap_limit_stall_across_committed_batch) is the GREEN guard
    under his mechanism; our identical-scenario port is superseded (same
    path, and its setup assumed our retention machinery).
  - APPLIED PR dashpay#851 (dashpay#649 out-of-order spend fix; the 27-commit linear
    chain only, excluding the dev merges it carried — dashpay#867/dashpay#868/dashpay#877/…).
  - DEFAULT_COINJOIN_GAP_LIMIT 100 -> 30, doc rewritten to state the
    experiment honestly.
  - PRESERVED the asset-lock router fix (CoinJoin + DashPay in the
    AssetLock arm of get_relevant_account_types) and rust-dashcore dashpay#863.

Companion test change (packages/rs-platform-wallet/.../asset_lock/build.rs):
coinjoin_gap_limit_discovers_addresses_beyond_the_old_window now tracks
DEFAULT_COINJOIN_GAP_LIMIT (asserts cj.gap_limit() == the constant and
probes far_index = constant - 1) instead of hardcoding 100 / index 50, so
it stays a valid far-edge discovery regression check at any gap; adds
coinjoin_gap_limit_is_experiment_value_30 pinning the compiled constant.

Verification: cargo test green — dash-spv 475, key-wallet 569,
key-wallet-manager 46 (in the standalone clone composite); platform-wallet
438. AAR republished to mavenLocal (org.dashj:dash-sdk-android:0.1.0-SNAPSHOT);
natives carry rescan_committed_range symbols and no drain_committed_rescans.
Known-good dashpay#873/gap-100 AAR backed up at 0.1.0-SNAPSHOT.bak-873-gap100.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants