Skip to content

Conversation

@igor-aptos
Copy link
Contributor

@igor-aptos igor-aptos commented Dec 21, 2025

Description

How Has This Been Tested?

Key Areas to Review

Type of Change

  • New feature
  • Bug fix
  • Breaking change
  • Performance improvement
  • Refactoring
  • Dependency update
  • Documentation update
  • Tests

Which Components or Systems Does This Change Impact?

  • Validator Node
  • Full Node (API, Indexer, etc.)
  • Move/Aptos Virtual Machine
  • Aptos Framework
  • Aptos CLI/SDK
  • Developer Infrastructure
  • Move Compiler
  • Other (specify)

Checklist

  • I have read and followed the CONTRIBUTING doc
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I identified and added all stakeholders and component owners affected by this change as reviewers
  • I tested both happy and unhappy path of the functionality
  • I have made corresponding changes to the documentation

Note

Introduces a dedicated trading types package and migrates order book code to it.

  • New bulk_order_types, bulk_order_utils modules with docs/tests; moves creation, accessors, and matching/reinsertion helpers out of bulk_order_book and legacy types
  • Replace order_book_types::OrderIdTypeOrderId and UniqueIdxTypeIncreasingIdx across active_order_book, bulk_order_book, and related docs
  • Update bulk_order_book APIs to use order_match_types and bulk_order_utils (match_order_and_get_next_from_bulk_order, reinsert_order_into_bulk_order), and to source data via order.get_order_request()
  • Switch big map helpers to order_book_utils::new_default_big_ordered_map; adjust dead man’s switch ops/tracker to new helpers and types
  • Add aptos_trading address in Move.toml

Written by Cursor Bugbot for commit 7b6a223. This will update automatically on new commits. Configure here.

@igor-aptos igor-aptos requested a review from wrwg as a code owner December 21, 2025 07:20
@igor-aptos igor-aptos marked this pull request as draft December 21, 2025 07:20
@igor-aptos igor-aptos force-pushed the igor/add_trading_package_for_types branch 5 times, most recently from 70a69bf to 48b43f1 Compare January 9, 2026 18:50
@igor-aptos igor-aptos changed the title [wip] add trading framework package for types add trading framework package for types Jan 9, 2026
@igor-aptos igor-aptos marked this pull request as ready for review January 9, 2026 18:55
<pre><code>enum <a href="bulk_order_book_types.md#0x7_bulk_order_book_types_BulkOrderPlaceResponse">BulkOrderPlaceResponse</a>&lt;M: <b>copy</b>, drop, store&gt; <b>has</b> <b>copy</b>, drop
=======
<pre><code><b>struct</b> <a href="bulk_order_types.md#0x7_bulk_order_book_types_BulkOrderPlaceResponse">BulkOrderPlaceResponse</a>&lt;M: <b>copy</b>, drop, store&gt; <b>has</b> <b>copy</b>, drop
>>>>>>> f583b0f4e1 (add trading framework package for types)
Copy link

Choose a reason for hiding this comment

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

Git merge conflict markers left in documentation

High Severity

Unresolved git merge conflict markers (<<<<<<<, =======, >>>>>>>) were accidentally committed to the documentation file bulk_order_book_types.md. These appear in at least four locations (lines 360-364, 433-437, 856-862, and 892-902) showing conflicts between enum vs struct declarations and different module references (bulk_order_book_types vs bulk_order_types). This breaks the documentation and indicates the merge was not properly resolved before committing.

Additional Locations (2)

Fix in Cursor Fix in Web

@igor-aptos igor-aptos force-pushed the igor/add_trading_package_for_types branch from 48b43f1 to 7b6a223 Compare January 19, 2026 06:09
@igor-aptos igor-aptos enabled auto-merge (squash) January 19, 2026 06:17
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

✅ Forge suite compat success on d49896be27f4c071cdaeb4a2e6ddc0ddd97a93c5 ==> 7b6a223ac430c92f9f128c9d76d6cf30ab8059d1

Compatibility test results for d49896be27f4c071cdaeb4a2e6ddc0ddd97a93c5 ==> 7b6a223ac430c92f9f128c9d76d6cf30ab8059d1 (PR)
1. Check liveness of validators at old version: d49896be27f4c071cdaeb4a2e6ddc0ddd97a93c5
compatibility::simple-validator-upgrade::liveness-check : committed: 10924.63 txn/s, latency: 2590.84 ms, (p50: 2500 ms, p70: 2700, p90: 2900 ms, p99: 12900 ms), latency samples: 469940
2. Upgrading first Validator to new version: 7b6a223ac430c92f9f128c9d76d6cf30ab8059d1
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 5896.95 txn/s, latency: 5727.94 ms, (p50: 6400 ms, p70: 6400, p90: 6500 ms, p99: 6600 ms), latency samples: 204220
3. Upgrading rest of first batch to new version: 7b6a223ac430c92f9f128c9d76d6cf30ab8059d1
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 5743.55 txn/s, latency: 5885.09 ms, (p50: 6600 ms, p70: 6600, p90: 6700 ms, p99: 6800 ms), latency samples: 199280
4. upgrading second batch to new version: 7b6a223ac430c92f9f128c9d76d6cf30ab8059d1
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 9538.64 txn/s, latency: 3554.32 ms, (p50: 3800 ms, p70: 3900, p90: 4100 ms, p99: 4200 ms), latency samples: 311600
5. check swarm health
Compatibility test for d49896be27f4c071cdaeb4a2e6ddc0ddd97a93c5 ==> 7b6a223ac430c92f9f128c9d76d6cf30ab8059d1 passed
Test Ok

@github-actions
Copy link
Contributor

✅ Forge suite realistic_env_max_load success on 7b6a223ac430c92f9f128c9d76d6cf30ab8059d1

two traffics test: inner traffic : committed: 13559.27 txn/s, latency: 2777.90 ms, (p50: 2700 ms, p70: 2900, p90: 3000 ms, p99: 3600 ms), latency samples: 5048860
two traffics test : committed: 100.03 txn/s, latency: 885.79 ms, (p50: 800 ms, p70: 800, p90: 900 ms, p99: 5300 ms), latency samples: 1720
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 2.218, avg: 2.147", "ConsensusProposalToOrdered: max: 0.170, avg: 0.167", "ConsensusOrderedToCommit: max: 0.052, avg: 0.047", "ConsensusProposalToCommit: max: 0.220, avg: 0.214"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.48s no progress at version 23677 (avg 0.07s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.69s no progress at version 2347352 (avg 0.69s) [limit 16].
Test Ok

@github-actions
Copy link
Contributor

✅ Forge suite framework_upgrade success on d49896be27f4c071cdaeb4a2e6ddc0ddd97a93c5 ==> 7b6a223ac430c92f9f128c9d76d6cf30ab8059d1

Compatibility test results for d49896be27f4c071cdaeb4a2e6ddc0ddd97a93c5 ==> 7b6a223ac430c92f9f128c9d76d6cf30ab8059d1 (PR)
Upgrade the nodes to version: 7b6a223ac430c92f9f128c9d76d6cf30ab8059d1
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 2322.01 txn/s, submitted: 2330.14 txn/s, failed submission: 8.13 txn/s, expired: 8.13 txn/s, latency: 1249.58 ms, (p50: 1200 ms, p70: 1400, p90: 1700 ms, p99: 2300 ms), latency samples: 211423
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 2267.93 txn/s, submitted: 2275.01 txn/s, failed submission: 7.08 txn/s, expired: 7.08 txn/s, latency: 1266.73 ms, (p50: 1200 ms, p70: 1500, p90: 1800 ms, p99: 2400 ms), latency samples: 205061
5. check swarm health
Compatibility test for d49896be27f4c071cdaeb4a2e6ddc0ddd97a93c5 ==> 7b6a223ac430c92f9f128c9d76d6cf30ab8059d1 passed
Upgrade the remaining nodes to version: 7b6a223ac430c92f9f128c9d76d6cf30ab8059d1
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 1525.28 txn/s, submitted: 1531.23 txn/s, failed submission: 5.95 txn/s, expired: 5.95 txn/s, latency: 1912.55 ms, (p50: 1200 ms, p70: 1500, p90: 1800 ms, p99: 11800 ms), latency samples: 138461
Test Ok

@igor-aptos igor-aptos merged commit b0102f4 into main Jan 19, 2026
89 of 90 checks passed
@igor-aptos igor-aptos deleted the igor/add_trading_package_for_types branch January 19, 2026 17:41
@igor-aptos
Copy link
Contributor Author

left only the move changes here, and left creating the new package for separate PR, as I need to figure out the setup for it

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.

4 participants