Repository navigation
feat(orderbook): read how much a market traded in a time window - #140
Conversation
Time Submission Status
Submit or update total time with: Add time on top of previous submission with: See available commands to help comply with our Guidelines. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Linked Issues check |  |
The changes implement the main [#139] coding objectives. They add TNClient.get_market_activity, MarketActivity, the sdk-go pin, the Go binding, type-preserving JSON conversion, error propagation, … |
Reconcile the action-based implementation with [#139]. Update the API documentation to state the actual node and private-mode requirements, including why the ad hoc-query requirement does or does not apply. Add focused test evidence for the… |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly summarizes the main change: adding market activity data for a specified time window. It is concise and directly related to the changeset. Out of Scope Changes check The dependency update, Go binding, Python API and type, documentation, unit tests, live tests, and type-existence checks all support [ #139]. The reviewed summary shows no node changes, aggregation hel…Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 6 files.
Full details: Linked Issues check
-
Explanation
The changes implement the main [
#139] coding objectives. They addTNClient.get_market_activity,MarketActivity, the sdk-go pin, the Go binding, type-preserving JSON conversion, error propagation, tests, and API documentation for bridge volume and coverage. The API documentation does not include the directly requested note that the call requires a node that allows ad hoc queries. The PR description explains the omission, but it does not satisfy the open issue requirement. The available evidence also does not show a focused private-mode error check.Resolution
Reconcile the action-based implementation with [
#139]. Update the API documentation to state the actual node and private-mode requirements, including why the ad hoc-query requirement does or does not apply. Add focused test evidence for the resulting private-mode behavior and error propagation.
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
The Docker distribution check builds the SDK wheel in one image and installs it in a clean one, then imports the SDK, creates a client against the mainnet gateway and derives an account. Both images now start from `python:3.12-slim-bookworm` (Debian 12) instead of `python:3.12-slim-bullseye` (Debian 11). The package list, Go 1.24.1 and gopy v0.4.10 stay as they were. The smoke script those steps run, `tests/distribution/get_account_test.py`, now ends with exit code 1 when a step fails. It printed the failure and still exited 0 before, so the Docker job and both native build jobs passed whatever it printed. A client that cannot reach a node still passes, as before, because the native jobs run without one. The Docker job talks to the mainnet gateway, so a gateway error other than an unreachable host now shows as a failed job. Release wheels are built on the GitHub runner by `release-build.yml`, which never uses this image or the smoke script, so nothing that ships changes. ## Why Debian 11's long term support ended on 31 August 2026, and its security updates moved from deb.debian.org to archive.debian.org. The package index on deb.debian.org still lists those updates, so `apt-get install` on Debian 11 asks for files the mirror no longer serves and stops with a 404. Both stages install packages with apt, so both stages move to Debian 12. Debian 12 has long term support until 30 June 2028. The check first stopped at the apt step on this pull request: - #140 ## Testing - On the stock images, installing the packages this Dockerfile uses (curl, git, build-essential, patchelf) stops with 404s from debian-security on `python:3.12-slim-bullseye` and completes on `python:3.12-slim-bookworm`. - I built the image from a shallow clone of this branch with the workflow's `docker build` command, then ran it with `TN_ENDPOINT` set to the mainnet gateway, as the workflow does. The wheel built and installed, the script printed all three steps as passed (the import, the client, and the account), and `docker run` returned 0. - I ran each path of the smoke script with the old and the new version. Against the mainnet gateway, and with no node reachable, both versions pass and exit 0. Four failure paths exited 0 before and exit 1 now: the SDK not installed, another error while importing it, a bad private key when creating the client, and a failing account call. - In the image, `docker run` returns 1 with a bad private key and 0 with no node reachable. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Chores** * Updated the distribution container image to a newer Debian-based Python image. * Distribution checks now report failures with a nonzero exit status for import, account-derivation, and unexpected client-creation errors. Connection-refused and dial errors return a success status. * These updates affect distribution packaging and test results; they do not change end-user functionality. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
@holdex pr submit-time 4h |
Description
A Python user can now ask how much traded on one order book over a time window.
client.get_market_activity(query_id, from_ts, to_ts)returns aMarketActivitydict with the filled volume and its direct and mint and burn parts, the unique traders, the fill and share counts, the first and last fill inside the window, and how far back the node's order events reach.The call goes through sdk-go's
OrderBook.GetMarketActivity, which reads the node actionget_market_activity. The node action holds the definition of volume, so Python reads the same figures as Go and JavaScript, and a change to the definition needs no SDK release. The action and the other two SDKs' methods come from:What this adds:
go.modpins sdk-go v0.7.8, the release that carriesGetMarketActivity. Only the sdk-go lines change, and kwil-db stays where it was.GetMarketActivityinbindings/bindings.gocalls sdk-go and returns one JSON object whose keys are the action's column names. The three cents fields stay strings becauseNUMERIC(78,0)exceeds what a JSON number holds exactly, and the fill times arenullwhen nothing traded. sdk-go checks the arguments and reports a missing market, and the binding passes both errors on, so Python raisesRuntimeErrorwith sdk-go's message in it.TNClient.get_market_activityand theMarketActivityTypedDict inclient.py, next toget_full_market_depth.docs/api-reference.mdwith the key table, an example, and the notes on bridges and coverage.The linked issue describes ad hoc SQL running in sdk-go. sdk-go calls the node action instead, as its pull request above explains. For Python nothing changes, since the binding, the dict and its keys are the ones the issue specifies.
The issue also asks for two notes in the docs, both written for the ad hoc SQL. Neither applies to the action, so I left both out, as the Go and JavaScript docs do:
TNClientis created with.(query_id, block_timestamp), the fix the issue points to.Where it works
Mainnet has carried
get_market_activitysince 8 October, and testnet since 13 October.Related Problem
Part of:
How Has This Been Tested?
tests/test_market_activity.pymonkeypatches the binding, astests/test_full_market_depth.pydoes, so it needs no node. It checks that the three arguments reach the binding in order, that the dict decodes to every field, that cents above 2**63 stay exact strings, thatnullfill times and a false coverage survive, that an error from the binding propagates, and that an empty answer raises instead of reading as no activity.bindings/market_activity_test.gochecks the JSON the binding builds: the action's column names, cents abovemath.MaxInt64as strings, andnullfill times with block 0 and false coverage when nothing traded.tests/test_order_book.pyinclude the new method and type.I broke the change in 15 ways, one at a time: 6 in the Python wrapper (argument order, the market id, an empty answer read as no activity, a swallowed error, cents turned into numbers, fill times defaulted to 0), 8 in the JSON the binding builds (a key name, cents as a JSON number, each fill time, the coverage fields, a count read from the wrong field), and the window arguments swapped inside the binding. A test failed every time. Only a live run can catch the last one, and the live suite did.
tests/test_market_activity_live.pyruns whenTN_LIVE_NODE_URLis set, like the other live suites, so CI skips it. It finds the newest market with a fill in the last 30 days and checks thatvolume_centsequalsdirect_cents + mint_burn_cents, thatdirect_fill_count <= fill_count, that the fills fall inside the window, that both ends of the window are inclusive, and that a market that does not exist raises.Called directly, mainnet market 1835 over [1788891555, 1791483555] returns 1145 cents (545 direct, 600 mint and burn), 2 traders, 6 fills (4 direct) and 20 shares with complete coverage, the figures the Go and JavaScript methods read. An inverted window and
query_id0 raiseRuntimeErrorwith sdk-go's messages.make gopy_build,go build,go vetandgofmtare clean. The 157 tests in the 11 test files that need no node pass. pyright reports the same 2 errors onclient.pyas on main, and none new. The tests that need a Docker node run in CI.Summary by CodeRabbit