Skip to content

Conversation

ffranr
Copy link
Contributor

@ffranr ffranr commented Aug 29, 2025

Closes: #1773


This PR updates documentation and comments for RFQ order handling:

  1. rfqrpc:

    • Update peer_pub_key field comments in BuyOrderRequest and SellOrderRequest to reflect that the field is required, removing the misleading "optional" wording.
  2. rpcserver:

    • Simplify comments.
    • Fix incorrect reference where a buy order was mentioned instead of a sell order.

See also: #1773 (comment)

ffranr added 2 commits August 29, 2025 00:52
Remove misleading "optional" wording from the `peer_pub_key` field
comments in BuyOrderRequest and SellOrderRequest proto messages.
The updated comments now accurately describe the field as required.
Simplify comments in the RPC server code and correct an error where a
comment incorrectly referred to a buy order instead of a sell order.
@ffranr ffranr self-assigned this Aug 29, 2025
@ffranr ffranr added bug Something isn't working gRPC RFQ Work relating to TAP channel Request For Quote (RFQ). labels Aug 29, 2025
@ffranr ffranr requested review from jtobin and GeorgeTsagk August 29, 2025 00:04
@ffranr ffranr moved this from 🆕 New to 👀 In review in Taproot-Assets Project Board Aug 29, 2025
@coveralls
Copy link

coveralls commented Aug 29, 2025

Pull Request Test Coverage Report for Build 17310845791

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 26 unchanged lines in 7 files lost coverage.
  • Overall coverage increased (+8.0%) to 56.666%

Files with Coverage Reduction New Missed Lines %
fn/context_guard.go 1 91.94%
tapdb/mssmt.go 2 89.55%
tapdb/multiverse.go 2 79.52%
tapgarden/custodian.go 4 76.83%
universe/archive.go 5 80.52%
itest/assertions.go 6 88.93%
mssmt/compacted_tree.go 6 77.97%
Totals Coverage Status
Change from base Build 17301129198: 8.0%
Covered Lines: 61007
Relevant Lines: 107661

💛 - Coveralls

Copy link
Member

@jtobin jtobin left a comment

Choose a reason for hiding this comment

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

Nice catches, LGTM. 👍

@ffranr ffranr enabled auto-merge August 29, 2025 09:46
@ffranr ffranr added this pull request to the merge queue Sep 3, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 3, 2025
@ffranr ffranr added this pull request to the merge queue Sep 3, 2025
Merged via the queue into main with commit 21fb82b Sep 3, 2025
19 checks passed
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Taproot-Assets Project Board Sep 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working gRPC RFQ Work relating to TAP channel Request For Quote (RFQ).
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

[bug]: AddAssetSellOrder requires peer_pub_key whiles mentions it as optional in the proto docs
4 participants