docs: removeOrder3 emits RemoveOrderV3 only on state change#62
docs: removeOrder3 emits RemoveOrderV3 only on state change#62thedavidmeister wants to merge 1 commit into
Conversation
The removeOrder3 NatSpec said the event "will be emitted" for every call, including redundant or never-existing removals. That contradicts the @return stateChanged doc, which returns false when the order did not exist. Reconcile the prose with the no-op semantics: a RemoveOrderV3 event is emitted, and state changes, only when a live order is actually removed; an already-dead or never-existing removal is a no-op that returns false. This mirrors the addOrder4 spec. Closes #2669 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 14 minutes and 26 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
What
Doc-only fix to the
removeOrder3NatSpec inIRaindexV6.sol. This isthe authoritative source consumed via
@inheritdocby the raindeximplementation.
Why
The prose said removing an order multiple times or removing one that
never existed is valid and "the event will be emitted". That
unconditional claim contradicts the
@return stateChangeddoc on thesame function ("false if it did not exist") and the actual code, which
guards the
RemoveOrderV3emit (and post tasks) behind a live-ordercheck: a dead / never-existing removal is a no-op that returns false and
emits nothing.
Per the maintainer decision on rainlanguage/raindex#2669 ("emit events
when there is a state change; update the docs to match the code"), this
reconciles the prose with the no-op semantics:
RemoveOrderV3, andreturn true.
state or emit an event, and returns false.
This mirrors the symmetric
addOrder4spec a few lines above. The@return stateChangeddoc is unchanged and now consistent with theprose. Deprecated v1-v4 interfaces are intentionally left untouched.
Testing
Doc-only change.
forge buildsucceeds andforge fmt --checkisclean. A NatSpec comment has no observable runtime behavior and this
interface-only package has no test harness or doc-gen in CI, so there is
no compiled artifact a test could assert against; no behavioral test is
applicable.
Resolves rainlanguage/raindex#2669
🤖 Generated with Claude Code