Repository navigation
fix(#4): match exact deployed payments and retain refusals - #609
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Independent reviewer: Boole ( Source approved. Boole independently read the complete payment/refusal source and its test changes, then verified both main integrations. The feature patch is byte-identical after #603's URL updates and #604's two requirement lower-bound updates. No source blocker remains. Boole verified the complete local evidence at The author additionally ran the recorded focused authentication/payment/submission/signable controls with PyJWT2.14.0 at this final head:129 tests/16 dedicated-scoped skips,69.299s,exit0. Full current-head hosted CI and final evidence review are still required before merge. This comment does not claim a merge slot or issue completion. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58cf9853e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Independent reviewer: Boole ( Source and final validation approved. No review blockers remain. Boole independently inspected the final ordinary log, completed backend/workspace/gate logs, full schema artifact and current check snapshot. The source approval recorded in5681274425 remains at this same head. CI34977272575 passed ordinary 3,876 tests /438 expected dedicated-suite skips,1301.767s; scoped 369/0; EVM 63/0; Bitcoin 4/0; real services 15/0; shared/dashboard/mobile/contracts 380/276/567/138; gates 294; Chromium smoke 3. Complete schema matches. Lint, migration drift, migrations and roles/catalogue pass. Hosted dependencies include PyJWT2.14.0 and openai3.14.0. All ten check results completed: eight successes and the two intended Android/iOS scope skips. Logs identify the reviewed head and expected base; the worktree is clean. Approval covers this payment/refusal correction. Native device execution, live deployment and real-value use were not exercised; remaining #4/#6/#5 acceptance stays open. |
|
Merge held: new behavioral review finding During the final review-comment audit I found the automated P1 in4016347031: an existing open candidate whose proposed fill is unrepresentable can repeatedly refuse incoming submissions before later valid candidates are considered. The final hosted validation and Boole review above establish the recorded tests, but do not clear this additional matching behavior. Do not merge this head while it is being assessed. No merge slot has been claimed. I have reopened independent behavioral review with Boole and am checking the actual create/matching path. The owner-selected permanent outcome for a refused submission UUID remains in force. Any correction must preserve that original outcome while resolving candidate selection explicitly, with a regression that fails at this head. #4 remains the owning issue. |
Attempt candidates in existing price/time order through the atomic match boundary. Roll back only unrepresentable proposed fills, preserve permanent all-invalid submission refusals, and retain the original UUID outcome after a valid candidate arrives. Keep one deployment snapshot per actual settlement attempt.
RonildoBraga
changed the title
|
@codex review Please re-review head Independent reviewer Boole approved source and local evidence at The source/local hold is cleared by Boole; your original finding and fresh current-head hosted checks still need clearance before merge. No merge slot is claimed yet. |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Candidate-selection hold cleared at The original automated reviewer completed the explicitly requested re-review at this head and reported no major issues in 5688077801, with its approval reaction timestamped 21:05:28Z. Boole's independent source/local approval is recorded in5688030077. Together these clear the behavioral hold I recorded in5681696628. Fresh hosted verification is still pending in the two Django jobs of run35023093433. Source gates, JavaScript/workspace/contract checks, metadata, security and native scope/aggregate checks have completed successfully; native builds were scoped out. No merge or slot claim until both Django jobs and final evidence review finish. |
|
Independent reviewer: Boole ( Boole independently verified current-head hosted evidence: ordinary 3,888 tests / 444 expected skips, 1228.365s; scoped 375 / 0, 337.770s; EVM 63 / 0; Bitcoin 4 / 0; Redis/ClamAV 15 / 0; workspace suites 380 / 276 / 567 / 138; Chromium smoke 3; gates 294. Formatting, migrations, role/catalogue checks and complete schema comparison passed. The final snapshot has ten completed checks: eight successful and two native builds skipped by scope. CI's merge tree equals the reviewed head; the worktree is clean and includes current main. The candidate-selection finding is cleared, including the original automated reviewer's fresh exact-head review in5688077801. Review covered source behavior, regression strength and local/hosted evidence. Boole did not rerun tests or exercise native devices. This clearance covers #609's scope; the remaining #4/#6/#5 settlement-recovery work stays open. |
Refs #4
Matched payments now use the captured deployment's decimals and exact integer units. Three shares at 1.23 with two deployed decimals produce 369 raw units even when asset pricing uses six decimals. Inexact totals and amounts outside the stored range cannot persist a failed match.
Matching tries compatible candidates in existing price/time order until a proposed fill is representable. Each failed attempt rolls back before the next; skipped resting orders, reservations and timestamps stay unchanged. No fill is rounded or resized. If every compatible candidate fails, the submission UUID records a permanent refusal. With no compatible candidate, admission remains OPEN. A refused UUID never becomes successful after a valid candidate arrives; a deliberate new attempt requires a new UUID. This implements the owner's selected policy and addresses the candidate-selection finding.
Calculation and immutable context capture share one deployment snapshot per actual attempt. Exact formatting replaces the float helper; completed V1 market reads decode the recorded scale. Existing signed history remains unchanged, including older V1 amounts that disagree with their quote.
tokens.0055_order_submission_settlement_refusalextends the existing refusal constraint; upgrade preserves history and reversal refuses to discard a newly supported refusal. No RLS policy or API response-shape change.Local evidence
Source
965df4b1ebc57e9de953ccead36a815b85021006, isolated PostgreSQL 16 and synthetic chains:settings.test --parallel 4 --noinputsettings.test_scoped --require-scoped-coverage --noinputmake testmake test-gatesmake check,make lint, backend Black/isort/flake8 andgit diff --checkpassedThe six new candidate controls failed against the previous matcher (six failures, 5.781s), then passed. They cover both incoming sides, exact whole lots with inexact partial fills, overflow, multiple rejected candidates, price/time selection, unchanged database and Python parent state, no skipped-attempt events, immutable refusal replay, non-amount failures and the winning deployment snapshot. They also run in the registered scoped suite.
A broader focused run exposed a test expecting 500 where DatabaseError correctly maps to 503. Its server-error assertion was corrected while retaining the pending/unspent/rollback checks. That run and the interrupted earlier scoped run are superseded. Earlier arithmetic and permanent-refusal controls also failed before their fixes. The signed-history control passes; restoring the old decoder produces the expected wrong-scale failure. A previously corrected large-integer fixture now sets the deployment scale and retains its exact raw-amount assertion.
Local dependencies include PyJWT 2.14.0, openai 3.13.0, Django 6.1.1 and web3 8.0.0. The earlier EVM run with PyJWT 2.13.0 is excluded; the table uses the rerun with 2.14.0.
Final head
fef67a3c1193e75ba86d5cea9dd77e5f5b7a8691integrates #602 (74a59145). Only the two marketing dependency files changed; the backend tree and complete feature patch are unchanged. Fresh marketing install, fullmake check, marketing lint/build and diff checking passed after integration. Independent reviewer Boole verified the source, final local evidence and integration delta and approved this exact head. He inspected the logs without rerunning the tests.Hosted verification
CI run 35023093433 passed at
fef67a3c1193e75ba86d5cea9dd77e5f5b7a8691; its synthetic mergea9f1427has the same complete source tree.5b5bd1d91525676ba010fc898865fe9496a1b08a8e628b610ff30e4dde32eef0Hosted runtime includes PostgreSQL 16.15, Django 6.1.1 and PyJWT 2.14.0. The dependency audit passed with its existing exclusions. All ten check results reported: eight successes and the two intended native build scope skips. Scope/aggregate checks passed; these skips do not establish a new Android/iOS build or physical-device result.
The original automated reviewer re-reviewed this exact head and reported no major issues; its matching finding is resolved. Boole independently verified the completed hosted logs and schema; final source/evidence signoff is recorded separately on this PR before merge. Prior run34977272575 covered the superseded
58cf9853head and is not used to establish the new candidate behavior.#4 remains open for legacy action retirement and #6 durable-execution/#5 reservation integration. Trading and signer admission stay closed. No live migration, deployment, activation or real-value use occurred. Full logs, source identities and failed/superseded evidence are retained under
.git/codex-checks/issue-4-exact-payment/.