rfq: add limit constraints and quote-level enforcement to rfq negotiation#2048
rfq: add limit constraints and quote-level enforcement to rfq negotiation#2048jtobin wants to merge 10 commits intolightninglabs:mainfrom
Conversation
Add a new optional TLV field (Type 29, TlvFixedPoint) to requestWireMsgData for carrying rate limit constraints on quote requests. This field supports both buy requests (minimum acceptable rate) and sell requests (maximum acceptable rate). Wire encode/decode follows the existing InAssetRateHint pattern.
Add AssetMinAmt and AssetRateLimit to BuyRequest, and PaymentMinAmt and AssetRateLimit to SellRequest. Update constructors, FromWire extraction, Validate (min <= max, rate positive), String output, and wire bridging functions. Add corresponding fields to BuyOrder and SellOrder structs and thread them through the negotiator's outgoing order handlers. Existing callers pass fn.None for both new params.
Add min fill and rate limit fields to RFQ and portfolio pilot proto definitions. Update rpcserver unmarshalling, portfolio pilot RPC marshal/unmarshal functions, and add new reject codes and quote response status values.
Add checkRateBound and checkMinFill enforcement to the internal portfolio pilot's VerifyAcceptQuote. Rate bound checks that the accepted rate satisfies the requester's limit (floor for buy, ceiling for sell). Min fill checks that the minimum amount is transportable at the accepted rate. Also adds MinFillNotMetRejectCode, PriceBoundMissRejectCode, and corresponding QuoteRespStatus values.
Add typed roundtrip tests for BuyRequest and SellRequest covering min fill, rate limit, and backward-compatible no-optional-field cases. Add validation tests for min > max and zero rate limit. Add rapid-based property tests covering wire roundtrip, min/max constraint validation, and rate bound enforcement semantics for both buy and sell requests. Extend TestVerifyAcceptQuote with rate bound enforcement cases (buy below/at/above limit, sell above/at limit) and min fill cases (zero msat, transportable). Add dedicated unit tests for checkRateBound and checkMinFill helpers. Update wire-level roundtrip test to cover AssetRateLimit TLV 29.
Add Validate() to NewBuyRequest and NewSellRequest constructors so that invalid requests (e.g. min > max) are caught locally before being sent over the wire. Add testRfqLimitConstraints: a pure RFQ integration test with five sub-tests exercising buy/sell orders with satisfied and violated rate limits, plus client-side min > max rejection. Add testCustomChannelsLimitConstraints: a custom channels smoke test that negotiates a sell quote with asset_rate_limit and payment_min_amt constraints over a real asset channel, then pays an invoice using the pre-negotiated quote.
Harden rate limit validation to reject negative coefficients (not just zero), add default cases to checkRateBound/checkMinFill type switches, fix stale godoc on unmarshalOptionalFixedPoint, path- anchor the gitignore entry, rewrite TestRateBoundEnforcementProperty to exercise wire roundtrip code, and add TestNegativeRateLimitRejected.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Request-for-Quote (RFQ) negotiation protocol by introducing client-side limit-order constraints. These changes allow users to define price boundaries and minimum fill requirements for their orders, which are then enforced during the quote acceptance process. The implementation is designed to be backward-compatible and includes robust validation logic to ensure that quotes violating these constraints are rejected with specific, machine-readable error codes. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces limit-order constraints for RFQ buy and sell orders, including new fields for minimum fill amounts and rate limits. The changes involve updating the RPC definitions, the RFQ manager, and the negotiator to enforce these constraints, along with comprehensive unit, property-based, and integration tests. I have no feedback to provide as the code changes are well-structured and the tests are thorough.
jtobin
force-pushed
the
limit-constraints
branch
from
b1f44e9 to
194d0c1
Compare
Fix gofmt alignment in rfq_test.go and request_property_test.go. Fix line-length violations in portfolio_pilot.go, portfolio_pilot_test.go, and request_property_test.go. Remove PaymentMinAmt from rate-limit-focused sell sub-tests: at the test oracle rate (1000 units/BTC), 1000 msat converts to zero asset units, tripping the checkMinFill guard.
1 participant
Partially resolves #2004 (roughly, workstreams A and B).
Adds limit order constraint fields to the RFQ wire protocol and RPC surface. Opus-generated TLDR for these changes:
For illustration of the kind of thing this enables, here's an example scenario from an e2e regtest demo of this (and related) functionality: