feat: GetSwapQuotes rpc
#624
Conversation
📝 WalkthroughWalkthroughAdds a GetSwapQuote RPC and client method, CLI command/handler, server routing for quote requests, fee-calculation utilities with tests, serializer mapping, a permission entry, and documentation updates exposing swap quote functionality with fee and limit checks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as CLI (boltzcli)
participant Client as boltzrpc.Client
participant Server as routedBoltzServer (gRPC)
participant PairStore as PairInfoStore
participant Utils as CalculateSwapQuote
CLI->>Client: GetSwapQuote(type, pair, send|receive)
Client->>Server: gRPC GetSwapQuote(request)
Server->>PairStore: GetPairInfo(type, pair)
PairStore-->>Server: PairInfo (fees, limits)
Server->>Utils: CalculateSwapQuote(type, send, receive, PairInfo.Fees)
Utils-->>Server: SwapQuote (send, receive, boltz_fee, network_fee)
Server->>Server: validate SwapQuote vs PairInfo.Limits
alt within limits
Server-->>Client: GetSwapQuoteResponse
Client-->>CLI: Response
else limit violation / invalid input
Server-->>Client: gRPC error
Client-->>CLI: error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
jackstar12
force-pushed
the
feat/swap-quotes
branch
from
c6e0007 to
00c8845
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
internal/rpcserver/router.go (1)
322-325: Confusing and unnecessary defaulting logic for SUBMARINE swap type.The condition
pair.From == boltzrpc.Currency_BTC && request.Pair != nil && request.Pair.From == 0is confusing and doesn't achieve the intended defaulting behavior:
- When
request.Pairis nil,pair.Fromis already 0 (BTC) by default (line 315), but the condition fails because it requiresrequest.Pair != nil- When
request.Pairis not nil, checking bothpair.From == boltzrpc.Currency_BTCandrequest.Pair.From == 0is redundant since they refer to the same value- Since
Currency_BTCis 0 (proto3 enum default), unspecified fields already default to BTC without this codeThe comment says "Default from to BTC if not specified", but the code doesn't match this intent. Consider either removing this code block entirely (since the default value is already BTC) or simplifying to:
// From defaults to BTC automatically (Currency_BTC is 0) // No explicit defaulting needed🔎 Suggested simplification
case boltzrpc.SwapType_SUBMARINE: // Submarine: on-chain → Lightning. To is always BTC (Lightning) pair.To = boltzrpc.Currency_BTC - if pair.From == boltzrpc.Currency_BTC && request.Pair != nil && request.Pair.From == 0 { - // Default from to BTC if not specified (Currency_BTC is 0) - pair.From = boltzrpc.Currency_BTC - } + // From defaults to BTC automatically since Currency_BTC is 0 (proto3 default)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pkg/boltzrpc/boltzrpc.pb.gois excluded by!**/*.pb.gopkg/boltzrpc/boltzrpc_grpc.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (10)
cmd/boltzcli/boltzcli.gocmd/boltzcli/commands.godocs/grpc.mdinternal/macaroons/permissions.gointernal/rpcserver/router.gointernal/utils/fees.gointernal/utils/fees_test.gopkg/boltzrpc/boltzrpc.protopkg/boltzrpc/client/client.gopkg/boltzrpc/serializers/utils.go
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-16T09:46:41.613Z
Learnt from: jackstar12
Repo: BoltzExchange/boltz-client PR: 487
File: internal/rpcserver/router.go:2304-2314
Timestamp: 2025-07-16T09:46:41.613Z
Learning: In the boltz-client codebase, RPC methods are protected by the macaroon permission system defined in internal/macaroons/permissions.go. Methods like GetSwapMnemonic and SetSwapMnemonic are already restricted to admin entities through the RPCServerPermissions map, so explicit isAdmin(ctx) checks are not needed in these methods.
Applied to files:
internal/rpcserver/router.gopkg/boltzrpc/boltzrpc.protointernal/macaroons/permissions.go
📚 Learning: 2025-05-26T09:32:25.533Z
Learnt from: jackstar12
Repo: BoltzExchange/boltz-client PR: 444
File: internal/rpcserver/router.go:2036-2052
Timestamp: 2025-05-26T09:32:25.533Z
Learning: In boltz-client, when handling Boltz backend connectivity during startup, the code intentionally uses different error handling strategies: logger.Fatalf for version incompatibility (fail fast approach) and retry logic for availability issues. Version incompatibility requires immediate human intervention and should not be retried.
Applied to files:
internal/utils/fees.go
🧬 Code graph analysis (4)
internal/utils/fees_test.go (3)
pkg/boltzrpc/boltzrpc.pb.go (6)
SwapFees(2063-2070)SwapFees(2085-2085)SwapFees(2100-2102)MinerFees(2181-2188)MinerFees(2203-2203)MinerFees(2218-2220)pkg/boltz/api.go (2)
Percentage(79-79)NormalSwap(29-29)internal/utils/fees.go (1)
CalculateSwapQuote(22-61)
internal/rpcserver/router.go (3)
pkg/boltzrpc/boltzrpc.pb.go (15)
GetSwapQuoteRequest(1169-1183)GetSwapQuoteRequest(1198-1198)GetSwapQuoteRequest(1213-1215)Pair(761-768)Pair(783-783)Pair(798-800)SwapType_SUBMARINE(181-181)SwapType_REVERSE(182-182)GetSwapQuoteRequest_SendAmount(1256-1259)GetSwapQuoteRequest_SendAmount(1266-1266)GetSwapQuoteRequest_ReceiveAmount(1261-1264)GetSwapQuoteRequest_ReceiveAmount(1268-1268)Fees(2236-2243)Fees(2258-2258)Fees(2273-2275)pkg/boltzrpc/serializers/utils.go (1)
ParseSwapType(48-57)internal/utils/fees.go (1)
CalculateSwapQuote(22-61)
pkg/boltzrpc/client/client.go (1)
pkg/boltzrpc/boltzrpc.pb.go (6)
GetSwapQuoteRequest(1169-1183)GetSwapQuoteRequest(1198-1198)GetSwapQuoteRequest(1213-1215)GetSwapQuoteResponse(1270-1285)GetSwapQuoteResponse(1300-1300)GetSwapQuoteResponse(1315-1317)
cmd/boltzcli/commands.go (2)
pkg/boltzrpc/boltzrpc.pb.go (32)
SwapType(178-178)SwapType(210-212)SwapType(214-216)SwapType(223-225)SwapType_SUBMARINE(181-181)SwapType_REVERSE(182-182)SwapType_CHAIN(183-183)Currency(132-132)Currency(161-163)Currency(165-167)Currency(174-176)Currency_BTC(135-135)Currency_LBTC(136-136)GetSwapQuoteRequest(1169-1183)GetSwapQuoteRequest(1198-1198)GetSwapQuoteRequest(1213-1215)Pair(761-768)Pair(783-783)Pair(798-800)GetSwapQuoteRequest_SendAmount(1256-1259)GetSwapQuoteRequest_SendAmount(1266-1266)GetSwapQuoteRequest_ReceiveAmount(1261-1264)GetSwapQuoteRequest_ReceiveAmount(1268-1268)PairInfo(1098-1107)PairInfo(1122-1122)PairInfo(1137-1139)Fees(2236-2243)Fees(2258-2258)Fees(2273-2275)Limits(2000-2008)Limits(2023-2023)Limits(2038-2040)internal/utils/utils.go (1)
Satoshis(16-18)
🪛 LanguageTool
docs/grpc.md
[style] ~1047-~1047: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...uint64 | | The amount you want to receive | #### GetSwapQuoteRespons...
(REP_WANT_TO_VB)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI (ubuntu-latest, 1.24.x)
🔇 Additional comments (10)
internal/macaroons/permissions.go (1)
76-79: LGTM! Appropriate permission level for the quote endpoint.The new RPC permission for
GetSwapQuotecorrectly usesEntity: "info"andAction: "read", consistent with other informational read-only endpoints likeGetPairInfoandGetPairs.pkg/boltzrpc/boltzrpc.proto (2)
31-35: LGTM! Clear RPC documentation.The RPC method documentation clearly describes the purpose and the either/or constraint for amount specification.
346-369: Well-structured protobuf messages.The
GetSwapQuoteRequestandGetSwapQuoteResponsemessages are well-designed:
oneofconstraint properly enforces mutual exclusivity ofsend_amountandreceive_amountat the protocol level- Response includes comprehensive information: amounts, fees breakdown, and pair limits
- Field naming follows protobuf conventions
cmd/boltzcli/boltzcli.go (1)
103-103: LGTM! Proper CLI command integration.The new
getSwapQuoteCommandis correctly added to the commands list and positioned logically alongside other query commands.pkg/boltzrpc/serializers/utils.go (1)
48-57: LGTM! Correct bidirectional mapping.The
ParseSwapTypefunction properly mirrors the existingSerializeSwapTypefunction, providing bidirectional conversion between gRPC and internal swap type representations.docs/grpc.md (1)
87-93: LGTM! Comprehensive API documentation.The GetSwapQuote endpoint is properly documented with clear descriptions of request and response fields. The documentation follows the established format and accurately reflects the protobuf definitions.
Note: This appears to be auto-generated documentation, which ensures consistency with the actual API.
Also applies to: 1037-1065
pkg/boltzrpc/client/client.go (1)
48-50: LGTM! Consistent client wrapper implementation.The
GetSwapQuotemethod follows the established pattern of other client wrapper methods, properly forwarding the request with context to the underlying gRPC client.internal/utils/fees_test.go (1)
12-94: LGTM! Comprehensive test coverage.The test suite thoroughly covers the
CalculateSwapQuotefunction:
- All three swap types (Normal/Submarine, Reverse, Chain)
- Both input modes (from send amount and from receive amount)
- Round-trip validation to ensure calculation consistency
The use of
InDeltawith a 1-satoshi tolerance in round-trip tests appropriately accounts for rounding in the float64 calculations. The test assertions verify all four fields of theSwapQuotestruct, ensuring complete coverage.internal/rpcserver/router.go (1)
340-384: LGTM!The rest of the implementation looks good:
- Amount extraction from the oneof field is handled correctly with proper error handling
- Quote calculation is delegated to the utils package
- Limit validation correctly determines which amount to check based on swap type (receive amount for SUBMARINE, send amount for REVERSE/CHAIN)
- Error messages are clear and informative
- Response construction includes all necessary fields (amounts, fees, and pair info)
cmd/boltzcli/commands.go (1)
100-211: LGTM! Clean and well-structured CLI implementation.The CLI implementation is excellent:
- Clear swap type parsing with helpful aliases (submarine/sub, reverse/rev, chain)
- Clean currency constraint logic that's much easier to understand than the server-side version:
- SUBMARINE: To is always BTC (Lightning), From defaults to BTC if not specified
- REVERSE: From is always BTC (Lightning), To defaults to LBTC if not specified
- CHAIN: Both currencies must be explicitly specified
- Good input validation ensuring exactly one of --send or --receive is provided
- Proper error messages for invalid inputs
- Nice output formatting with both JSON and human-readable options
- Fee breakdown clearly shows Boltz fee, network fee, and total
The currency defaulting approach here (checking if the string flag is empty) is clearer and more reliable than the server-side approach (checking if the proto field is 0).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
internal/utils/fees.go (1)
22-61: Division by zero risk when percentage fee >= 100%.Line 55 performs division by
(1 - rate). Iffees.Percentage >= 1.0(100% or higher), this results in division by zero or a negative denominator. While Boltz backend likely validates percentage ranges, adding defensive validation would prevent runtime panics:🔎 Suggested defensive check
func CalculateSwapQuote(swapType boltz.SwapType, sendAmount, receiveAmount uint64, fees *boltzrpc.SwapFees) *SwapQuote { + if fees.Percentage >= 1.0 { + panic("swap fee percentage must be less than 100%") + } percentage := boltz.Percentage(fees.Percentage) minerFee := fees.MinerFeesAlternatively, return an error instead of panicking if this function's signature can be updated.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pkg/boltzrpc/boltzrpc.pb.gois excluded by!**/*.pb.gopkg/boltzrpc/boltzrpc_grpc.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (10)
cmd/boltzcli/boltzcli.gocmd/boltzcli/commands.godocs/grpc.mdinternal/macaroons/permissions.gointernal/rpcserver/router.gointernal/utils/fees.gointernal/utils/fees_test.gopkg/boltzrpc/boltzrpc.protopkg/boltzrpc/client/client.gopkg/boltzrpc/serializers/utils.go
🚧 Files skipped from review as they are similar to previous changes (5)
- pkg/boltzrpc/client/client.go
- internal/utils/fees_test.go
- cmd/boltzcli/commands.go
- cmd/boltzcli/boltzcli.go
- pkg/boltzrpc/boltzrpc.proto
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-16T09:46:41.613Z
Learnt from: jackstar12
Repo: BoltzExchange/boltz-client PR: 487
File: internal/rpcserver/router.go:2304-2314
Timestamp: 2025-07-16T09:46:41.613Z
Learning: In the boltz-client codebase, RPC methods are protected by the macaroon permission system defined in internal/macaroons/permissions.go. Methods like GetSwapMnemonic and SetSwapMnemonic are already restricted to admin entities through the RPCServerPermissions map, so explicit isAdmin(ctx) checks are not needed in these methods.
Applied to files:
internal/macaroons/permissions.gointernal/rpcserver/router.go
📚 Learning: 2025-05-26T09:32:25.533Z
Learnt from: jackstar12
Repo: BoltzExchange/boltz-client PR: 444
File: internal/rpcserver/router.go:2036-2052
Timestamp: 2025-05-26T09:32:25.533Z
Learning: In boltz-client, when handling Boltz backend connectivity during startup, the code intentionally uses different error handling strategies: logger.Fatalf for version incompatibility (fail fast approach) and retry logic for availability issues. Version incompatibility requires immediate human intervention and should not be retried.
Applied to files:
internal/utils/fees.go
🧬 Code graph analysis (3)
internal/rpcserver/router.go (3)
pkg/boltzrpc/boltzrpc.pb.go (21)
GetSwapQuoteRequest(1169-1183)GetSwapQuoteRequest(1198-1198)GetSwapQuoteRequest(1213-1215)Pair(761-768)Pair(783-783)Pair(798-800)SwapType_SUBMARINE(181-181)SwapType_REVERSE(182-182)GetSwapQuoteRequest_SendAmount(1256-1259)GetSwapQuoteRequest_SendAmount(1266-1266)GetSwapQuoteRequest_ReceiveAmount(1261-1264)GetSwapQuoteRequest_ReceiveAmount(1268-1268)Fees(2236-2243)Fees(2258-2258)Fees(2273-2275)Limits(2000-2008)Limits(2023-2023)Limits(2038-2040)PairInfo(1098-1107)PairInfo(1122-1122)PairInfo(1137-1139)pkg/boltzrpc/serializers/utils.go (1)
ParseSwapType(48-57)internal/utils/fees.go (1)
CalculateSwapQuote(22-61)
pkg/boltzrpc/serializers/utils.go (2)
pkg/boltzrpc/boltzrpc.pb.go (6)
SwapType(178-178)SwapType(210-212)SwapType(214-216)SwapType(223-225)SwapType_SUBMARINE(181-181)SwapType_REVERSE(182-182)pkg/boltz/api.go (1)
NormalSwap(29-29)
internal/utils/fees.go (2)
pkg/boltzrpc/boltzrpc.pb.go (10)
SwapType(178-178)SwapType(210-212)SwapType(214-216)SwapType(223-225)SwapFees(2063-2070)SwapFees(2085-2085)SwapFees(2100-2102)MinerFees(2181-2188)MinerFees(2203-2203)MinerFees(2218-2220)pkg/boltz/api.go (2)
Percentage(79-79)NormalSwap(29-29)
🪛 LanguageTool
docs/grpc.md
[style] ~1047-~1047: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...uint64 | | The amount you want to receive | #### GetSwapQuoteRespons...
(REP_WANT_TO_VB)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI (ubuntu-latest, 1.24.x)
🔇 Additional comments (4)
internal/macaroons/permissions.go (1)
76-79: LGTM! Permission assignment is appropriate.The
info:readpermission level is correct for theGetSwapQuoteRPC, consistent with other read-only information queries likeGetPairInfoandGetPairs.pkg/boltzrpc/serializers/utils.go (1)
48-57: LGTM! Correct inverse mapping.The
ParseSwapTypefunction correctly mirrors the existingSerializeSwapTypelogic, providing proper bidirectional conversion between gRPC and internal swap type representations.docs/grpc.md (1)
87-93: LGTM! Documentation is clear and complete.The
GetSwapQuoteRPC documentation accurately describes the request/response structure and the fee breakdown functionality, providing users with the information needed to use this endpoint effectively.Also applies to: 1037-1065
internal/rpcserver/router.go (1)
352-368: LGTM! Limit validation logic is correct.The swap type-specific limit checks correctly validate:
- SUBMARINE: receive amount (Lightning invoice amount)
- REVERSE/CHAIN: send amount (Lightning/onchain payment amount)
This properly aligns with each swap type's fee structure and accurately enforces Boltz's operational limits.
jackstar12
force-pushed
the
feat/swap-quotes
branch
from
00c8845 to
177cf91
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
internal/rpcserver/router.go (1)
318-330: Silently overriding user-provided currency constraints may confuse callers.For SUBMARINE and REVERSE swaps, the code unconditionally overwrites
pair.To(line 321) andpair.From(line 324) even when the user explicitly provides aPairwith different values. This silent override could be confusing if a user specifies, for example,From = LBTCfor a REVERSE swap and the code changes it toBTCwithout notification.The past review comment on these lines suggested validating user-provided pairs against swap type constraints and returning an
InvalidArgumenterror on mismatch, rather than silently overriding. This would make the API more explicit and prevent user confusion.internal/utils/fees.go (1)
52-57: Division by zero risk remains unaddressed.The past review comment flagged that line 55's division by
(1 - rate)will fail whenfees.Percentage >= 1.0(100% or more). Although that comment was marked as addressed in commit 00c8845, no validation is present in the current code. Ifrate >= 1.0, the calculation produces a divide-by-zero panic (if rate == 1.0) or a negative denominator (if rate > 1.0), casting touint64and yielding incorrect results.Add validation:
🔎 Suggested bounds check
func CalculateSwapQuote(swapType boltz.SwapType, sendAmount, receiveAmount uint64, fees *boltzrpc.SwapFees) *SwapQuote { + if fees.Percentage < 0 || fees.Percentage >= 1.0 { + panic("percentage fee must be in range [0, 1)") + } percentage := boltz.Percentage(fees.Percentage)
🧹 Nitpick comments (2)
docs/grpc.md (1)
1037-1065: Consider minor rewording to reduce repetition.The field descriptions for
send_amountandreceive_amountboth use "The amount you want to..." which is slightly repetitive. Consider rewording one of them for variety.🔎 Suggested rewording
| Field | Type | Label | Description | | ----- | ---- | ----- | ----------- | | `type` | [`SwapType`](#swaptype) | | | | `pair` | [`Pair`](#pair) | | | -| `send_amount` | [`uint64`](#uint64) | | The amount you want to send | -| `receive_amount` | [`uint64`](#uint64) | | The amount you want to receive | +| `send_amount` | [`uint64`](#uint64) | | Amount to send (in satoshis) | +| `receive_amount` | [`uint64`](#uint64) | | Amount to receive (in satoshis) |cmd/boltzcli/commands.go (1)
124-134: Consider being explicit about the BTC default for submarine swaps.While the current code works correctly (uninitialized
fromdefaults toCurrency_BTCwhich is the zero value), being explicit about defaults improves code clarity and prevents future bugs if the enum ordering changes.🔎 Proposed change for clarity
case boltzrpc.SwapType_SUBMARINE: // Submarine: on-chain → Lightning. To is always BTC (Lightning) to = boltzrpc.Currency_BTC fromStr := ctx.String("from") if fromStr != "" { from, err = parseCurrency(fromStr) if err != nil { return err } + } else { + // Default to BTC for submarine swaps + from = boltzrpc.Currency_BTC }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pkg/boltzrpc/boltzrpc.pb.gois excluded by!**/*.pb.gopkg/boltzrpc/boltzrpc_grpc.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (10)
cmd/boltzcli/boltzcli.gocmd/boltzcli/commands.godocs/grpc.mdinternal/macaroons/permissions.gointernal/rpcserver/router.gointernal/utils/fees.gointernal/utils/fees_test.gopkg/boltzrpc/boltzrpc.protopkg/boltzrpc/client/client.gopkg/boltzrpc/serializers/utils.go
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/utils/fees_test.go
- pkg/boltzrpc/serializers/utils.go
- pkg/boltzrpc/boltzrpc.proto
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-16T09:46:41.613Z
Learnt from: jackstar12
Repo: BoltzExchange/boltz-client PR: 487
File: internal/rpcserver/router.go:2304-2314
Timestamp: 2025-07-16T09:46:41.613Z
Learning: In the boltz-client codebase, RPC methods are protected by the macaroon permission system defined in internal/macaroons/permissions.go. Methods like GetSwapMnemonic and SetSwapMnemonic are already restricted to admin entities through the RPCServerPermissions map, so explicit isAdmin(ctx) checks are not needed in these methods.
Applied to files:
internal/rpcserver/router.gointernal/macaroons/permissions.go
📚 Learning: 2025-05-26T09:32:25.533Z
Learnt from: jackstar12
Repo: BoltzExchange/boltz-client PR: 444
File: internal/rpcserver/router.go:2036-2052
Timestamp: 2025-05-26T09:32:25.533Z
Learning: In boltz-client, when handling Boltz backend connectivity during startup, the code intentionally uses different error handling strategies: logger.Fatalf for version incompatibility (fail fast approach) and retry logic for availability issues. Version incompatibility requires immediate human intervention and should not be retried.
Applied to files:
internal/utils/fees.go
🧬 Code graph analysis (4)
internal/rpcserver/router.go (3)
pkg/boltzrpc/boltzrpc.pb.go (15)
GetSwapQuoteRequest(1169-1183)GetSwapQuoteRequest(1198-1198)GetSwapQuoteRequest(1213-1215)Pair(761-768)Pair(783-783)Pair(798-800)SwapType_SUBMARINE(181-181)SwapType_REVERSE(182-182)GetSwapQuoteRequest_SendAmount(1256-1259)GetSwapQuoteRequest_SendAmount(1266-1266)GetSwapQuoteRequest_ReceiveAmount(1261-1264)GetSwapQuoteRequest_ReceiveAmount(1268-1268)Fees(2236-2243)Fees(2258-2258)Fees(2273-2275)pkg/boltzrpc/serializers/utils.go (1)
ParseSwapType(48-57)internal/utils/fees.go (1)
CalculateSwapQuote(22-61)
internal/utils/fees.go (2)
pkg/boltzrpc/boltzrpc.pb.go (10)
SwapType(178-178)SwapType(210-212)SwapType(214-216)SwapType(223-225)SwapFees(2063-2070)SwapFees(2085-2085)SwapFees(2100-2102)MinerFees(2181-2188)MinerFees(2203-2203)MinerFees(2218-2220)pkg/boltz/api.go (2)
Percentage(79-79)NormalSwap(29-29)
cmd/boltzcli/commands.go (1)
pkg/boltzrpc/boltzrpc.pb.go (31)
SwapType(178-178)SwapType(210-212)SwapType(214-216)SwapType(223-225)SwapType_SUBMARINE(181-181)SwapType_REVERSE(182-182)SwapType_CHAIN(183-183)Currency(132-132)Currency(161-163)Currency(165-167)Currency(174-176)Currency_BTC(135-135)GetSwapQuoteRequest(1169-1183)GetSwapQuoteRequest(1198-1198)GetSwapQuoteRequest(1213-1215)Pair(761-768)Pair(783-783)Pair(798-800)GetSwapQuoteRequest_SendAmount(1256-1259)GetSwapQuoteRequest_SendAmount(1266-1266)GetSwapQuoteRequest_ReceiveAmount(1261-1264)GetSwapQuoteRequest_ReceiveAmount(1268-1268)PairInfo(1098-1107)PairInfo(1122-1122)PairInfo(1137-1139)Fees(2236-2243)Fees(2258-2258)Fees(2273-2275)Limits(2000-2008)Limits(2023-2023)Limits(2038-2040)
pkg/boltzrpc/client/client.go (1)
pkg/boltzrpc/boltzrpc.pb.go (6)
GetSwapQuoteRequest(1169-1183)GetSwapQuoteRequest(1198-1198)GetSwapQuoteRequest(1213-1215)GetSwapQuoteResponse(1270-1285)GetSwapQuoteResponse(1300-1300)GetSwapQuoteResponse(1315-1317)
🪛 LanguageTool
docs/grpc.md
[style] ~1047-~1047: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...uint64 | | The amount you want to receive | #### GetSwapQuoteRespons...
(REP_WANT_TO_VB)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI (ubuntu-latest, 1.24.x)
🔇 Additional comments (7)
internal/macaroons/permissions.go (1)
76-79: LGTM!The permission entry for
GetSwapQuoteis correctly configured withEntity: "info"andAction: "read", consistent with other read-only information endpoints likeGetPairInfoandGetPairs.docs/grpc.md (1)
87-93: LGTM!The GetSwapQuote method documentation is clear and well-integrated into the existing documentation structure.
cmd/boltzcli/boltzcli.go (1)
103-103: LGTM!The command is properly registered in the application's command list, positioned logically after
getPairsCommand.pkg/boltzrpc/client/client.go (1)
48-50: LGTM!The
GetSwapQuotemethod correctly wraps the gRPC client call and follows the same pattern as other methods in this file.cmd/boltzcli/commands.go (3)
56-98: LGTM!The command definition is well-structured with clear documentation, appropriate flags, and helpful usage examples.
167-181: LGTM!The validation logic correctly ensures exactly one of
--sendor--receiveis specified, and properly sets the corresponding oneof field in the request.
188-206: LGTM!The output formatting is clear and well-structured, providing both JSON and human-readable formats with comprehensive fee breakdown and limit information.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
internal/rpcserver/router.go (1)
312-330: Pair initialization logic is correct but relies on implicit enum default.The code correctly applies currency constraints per swap type:
- SUBMARINE: Lightning receive (To = BTC)
- REVERSE: Lightning send (From = BTC)
- CHAIN: Both currencies must be explicit
The logic assumes the default Currency enum value (0) equals BTC, which is true per the protobuf definition. While this works, consider adding a comment to make this assumption explicit in case the enum changes in the future.
📝 Optional: Add clarifying comment
pair := request.Pair if pair == nil { + // Default Currency enum value (0) is BTC pair = &boltzrpc.Pair{} }internal/utils/fees.go (1)
41-51: Add default case to switch statements for defensive programming.The switch statements handle the three known swap types correctly. However, they lack a default case to handle unknown swap types. While the current call path ensures only valid types are passed (via
serializers.ParseSwapType), adding a default case would make the function more robust and self-documenting.🛡️ Proposed defensive programming enhancement
For the first switch (lines 41-51):
switch swapType { case boltz.NormalSwap: // Submarine: service fee on receive, so receive = (send - minerFee) / (1 + rate) rate := percentage.Ratio() quote.ReceiveAmount = uint64(float64(sendAmount-minerFee) / (1 + rate)) quote.BoltzFee = percentage.Calculate(quote.ReceiveAmount) case boltz.ReverseSwap, boltz.ChainSwap: // Reverse/Chain: service fee on send quote.BoltzFee = percentage.Calculate(sendAmount) quote.ReceiveAmount = sendAmount - quote.BoltzFee - minerFee + default: + return nil, fmt.Errorf("unsupported swap type: %s", swapType) }Apply similar change to the second switch (lines 55-66).
Also applies to: 55-66
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cmd/boltzcli/commands.gointernal/rpcserver/router.gointernal/utils/fees.gointernal/utils/fees_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- cmd/boltzcli/commands.go
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-16T09:46:41.613Z
Learnt from: jackstar12
Repo: BoltzExchange/boltz-client PR: 487
File: internal/rpcserver/router.go:2304-2314
Timestamp: 2025-07-16T09:46:41.613Z
Learning: In the boltz-client codebase, RPC methods are protected by the macaroon permission system defined in internal/macaroons/permissions.go. Methods like GetSwapMnemonic and SetSwapMnemonic are already restricted to admin entities through the RPCServerPermissions map, so explicit isAdmin(ctx) checks are not needed in these methods.
Applied to files:
internal/rpcserver/router.go
📚 Learning: 2025-05-26T09:32:25.533Z
Learnt from: jackstar12
Repo: BoltzExchange/boltz-client PR: 444
File: internal/rpcserver/router.go:2036-2052
Timestamp: 2025-05-26T09:32:25.533Z
Learning: In boltz-client, when handling Boltz backend connectivity during startup, the code intentionally uses different error handling strategies: logger.Fatalf for version incompatibility (fail fast approach) and retry logic for availability issues. Version incompatibility requires immediate human intervention and should not be retried.
Applied to files:
internal/utils/fees.go
🧬 Code graph analysis (3)
internal/utils/fees_test.go (3)
pkg/boltzrpc/boltzrpc.pb.go (6)
SwapFees(2063-2070)SwapFees(2085-2085)SwapFees(2100-2102)MinerFees(2181-2188)MinerFees(2203-2203)MinerFees(2218-2220)pkg/boltz/api.go (2)
Percentage(79-79)NormalSwap(29-29)internal/utils/fees.go (1)
CalculateSwapQuote(23-69)
internal/rpcserver/router.go (2)
pkg/boltzrpc/boltzrpc.pb.go (25)
GetSwapQuoteRequest(1169-1183)GetSwapQuoteRequest(1198-1198)GetSwapQuoteRequest(1213-1215)GetSwapQuoteResponse(1270-1285)GetSwapQuoteResponse(1300-1300)GetSwapQuoteResponse(1315-1317)Pair(761-768)Pair(783-783)Pair(798-800)SwapType_SUBMARINE(181-181)SwapType_REVERSE(182-182)SwapType_CHAIN(183-183)GetPairInfoRequest(1043-1050)GetPairInfoRequest(1065-1065)GetPairInfoRequest(1080-1082)GetSwapQuoteRequest_SendAmount(1256-1259)GetSwapQuoteRequest_SendAmount(1266-1266)GetSwapQuoteRequest_ReceiveAmount(1261-1264)GetSwapQuoteRequest_ReceiveAmount(1268-1268)Fees(2236-2243)Fees(2258-2258)Fees(2273-2275)PairInfo(1098-1107)PairInfo(1122-1122)PairInfo(1137-1139)internal/utils/fees.go (1)
CalculateSwapQuote(23-69)
internal/utils/fees.go (2)
pkg/boltzrpc/boltzrpc.pb.go (10)
SwapType(178-178)SwapType(210-212)SwapType(214-216)SwapType(223-225)SwapFees(2063-2070)SwapFees(2085-2085)SwapFees(2100-2102)MinerFees(2181-2188)MinerFees(2203-2203)MinerFees(2218-2220)pkg/boltz/api.go (2)
Percentage(79-79)NormalSwap(29-29)
🔇 Additional comments (5)
internal/utils/fees_test.go (1)
12-111: LGTM! Comprehensive test coverage.The test suite thoroughly covers:
- All three swap types (Normal, Reverse, Chain) with both input modes (from send/from receive)
- Round-trip consistency validation (within 1 sat delta due to rounding)
- Error conditions (both amounts specified, both zero)
The expected values align with the fee calculation logic in the implementation.
internal/rpcserver/router.go (2)
355-371: Correct limit validation logic.The handler correctly validates different amounts based on swap type:
- SUBMARINE checks
ReceiveAmount(Lightning/invoice side)- REVERSE/CHAIN check
SendAmountThis aligns with Boltz's limit semantics where submarine swap limits apply to the Lightning amount and reverse/chain limits apply to the initiating amount.
311-380: Well-structured quote calculation handler.The
GetSwapQuoteimplementation correctly:
- Applies type-specific currency constraints
- Extracts the input amount from the oneof field
- Delegates to
utils.CalculateSwapQuotefor calculations- Validates results against pair limits
- Returns a comprehensive response including PairInfo
Based on learnings, RPC methods are protected by the macaroon permission system defined in
internal/macaroons/permissions.go, so explicit permission checks aren't needed here.internal/utils/fees.go (2)
31-36: Input validation is correct.The function properly validates that exactly one of
sendAmountorreceiveAmountis provided, preventing ambiguous inputs.Minor: Line 32's error message uses "cant" instead of "can't", but this is acceptable for internal error messages.
38-66: Fee calculation logic is correct and user-friendly.The implementation correctly handles three swap types with appropriate fee models:
Submarine (NormalSwap):
- Fees applied to receive amount (you send X, fees deducted from what you receive)
- Truncation when calculating receive amount (conservative estimate)
Reverse/Chain:
- Fees applied to send amount (you want X, fees added to what you send)
- Uses
math.Ceilwhen calculating send amount (ensures sufficient payment)The asymmetric rounding (truncate when calculating receive, ceil when calculating send) is intentional and user-friendly—it ensures users always send enough and provides conservative receive estimates.
jackstar12
force-pushed
the
feat/swap-quotes
branch
from
9075ac5 to
27f8359
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @internal/utils/fees.go:
- Around line 38-51: The subtraction sendAmount - minerFee in the NormalSwap
branch can underflow when sendAmount < minerFee; update the logic in the block
handling boltz.NormalSwap (using sendAmount, minerFee, quote, percentage.Ratio,
percentage.Calculate) to first check if sendAmount <= minerFee and handle that
case explicitly (e.g., set quote.ReceiveAmount=0 and quote.BoltzFee=0 or return
an error/validation result), otherwise perform the subtraction and subsequent
division as before to compute quote.ReceiveAmount and quote.BoltzFee, ensuring
no uint64 underflow occurs.
- Around line 60-65: Detect and guard against a 100%+ fee ratio before dividing:
after obtaining rate := percentage.Ratio() in the boltz.ReverseSwap /
boltz.ChainSwap branch, check if rate >= 1.0 and handle it explicitly (e.g.,
return an error or propagate a failure) instead of performing quote.SendAmount =
uint64(math.Ceil(float64(receiveAmount+minerFee) / (1 - rate))); only compute
SendAmount and BoltzFee via percentage.Calculate(quote.SendAmount) when rate <
1.0 to avoid division by zero or negative divisors.
- Around line 47-51: In the ReverseSwap/ChainSwap branch (case
boltz.ReverseSwap, boltz.ChainSwap) the calculation quote.ReceiveAmount =
sendAmount - quote.BoltzFee - minerFee can underflow if quote.BoltzFee +
minerFee > sendAmount; update this block to compute totalFees := quote.BoltzFee
+ minerFee and then if totalFees >= sendAmount set quote.ReceiveAmount = 0 (or
return/propagate an explicit error), otherwise set quote.ReceiveAmount =
sendAmount - totalFees; ensure you use the same numeric types as
sendAmount/quote.BoltzFee/minerFee to avoid overflow/underflow and add a short
comment explaining the guard.
🧹 Nitpick comments (1)
internal/utils/fees.go (1)
41-51: Consider adding a default case for unknown swap types.The switch statement doesn't handle unknown swap types. If an unrecognized
swapTypeis passed, the function returns a quote with partial zero values without error.♻️ Suggested improvement
switch swapType { case boltz.NormalSwap: // Submarine: service fee on receive, so receive = (send - minerFee) / (1 + rate) rate := percentage.Ratio() quote.ReceiveAmount = uint64(float64(sendAmount-minerFee) / (1 + rate)) quote.BoltzFee = percentage.Calculate(quote.ReceiveAmount) case boltz.ReverseSwap, boltz.ChainSwap: // Reverse/Chain: service fee on send quote.BoltzFee = percentage.Calculate(sendAmount) quote.ReceiveAmount = sendAmount - quote.BoltzFee - minerFee + default: + return nil, fmt.Errorf("unsupported swap type: %s", swapType) }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pkg/boltzrpc/boltzrpc.pb.gois excluded by!**/*.pb.gopkg/boltzrpc/boltzrpc_grpc.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (10)
cmd/boltzcli/boltzcli.gocmd/boltzcli/commands.godocs/grpc.mdinternal/macaroons/permissions.gointernal/rpcserver/router.gointernal/utils/fees.gointernal/utils/fees_test.gopkg/boltzrpc/boltzrpc.protopkg/boltzrpc/client/client.gopkg/boltzrpc/serializers/utils.go
🚧 Files skipped from review as they are similar to previous changes (6)
- cmd/boltzcli/boltzcli.go
- pkg/boltzrpc/serializers/utils.go
- internal/utils/fees_test.go
- internal/rpcserver/router.go
- internal/macaroons/permissions.go
- pkg/boltzrpc/boltzrpc.proto
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-26T09:32:25.533Z
Learnt from: jackstar12
Repo: BoltzExchange/boltz-client PR: 444
File: internal/rpcserver/router.go:2036-2052
Timestamp: 2025-05-26T09:32:25.533Z
Learning: In boltz-client, when handling Boltz backend connectivity during startup, the code intentionally uses different error handling strategies: logger.Fatalf for version incompatibility (fail fast approach) and retry logic for availability issues. Version incompatibility requires immediate human intervention and should not be retried.
Applied to files:
internal/utils/fees.go
🧬 Code graph analysis (2)
cmd/boltzcli/commands.go (5)
pkg/boltzrpc/boltzrpc.pb.go (25)
SwapType(178-178)SwapType(210-212)SwapType(214-216)SwapType(223-225)SwapType_SUBMARINE(181-181)SwapType_REVERSE(182-182)SwapType_CHAIN(183-183)Currency(132-132)Currency(161-163)Currency(165-167)Currency(174-176)Currency_BTC(135-135)GetSwapQuoteRequest(1169-1183)GetSwapQuoteRequest(1198-1198)GetSwapQuoteRequest(1213-1215)Pair(761-768)Pair(783-783)Pair(798-800)GetSwapQuoteRequest_SendAmount(1256-1259)GetSwapQuoteRequest_SendAmount(1266-1266)GetSwapQuoteRequest_ReceiveAmount(1261-1264)GetSwapQuoteRequest_ReceiveAmount(1268-1268)Limits(2000-2008)Limits(2023-2023)Limits(2038-2040)pkg/boltz/api.go (2)
SwapType(26-26)Percentage(79-79)internal/logger/logger.go (1)
Errorf(97-99)internal/onchain/onchain.go (1)
Currency(77-80)internal/utils/utils.go (1)
Satoshis(16-18)
internal/utils/fees.go (2)
pkg/boltzrpc/boltzrpc.pb.go (10)
SwapType(178-178)SwapType(210-212)SwapType(214-216)SwapType(223-225)SwapFees(2063-2070)SwapFees(2085-2085)SwapFees(2100-2102)MinerFees(2181-2188)MinerFees(2203-2203)MinerFees(2218-2220)pkg/boltz/api.go (2)
Percentage(79-79)NormalSwap(29-29)
🪛 LanguageTool
docs/grpc.md
[style] ~1047-~1047: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...uint64 | | The amount you want to receive | #### GetSwapQuoteRespons...
(REP_WANT_TO_VB)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI (ubuntu-latest, 1.24.x)
🔇 Additional comments (5)
pkg/boltzrpc/client/client.go (1)
48-50: LGTM!The new
GetSwapQuotemethod follows the established pattern of other client wrapper methods in the file.cmd/boltzcli/commands.go (2)
56-98: LGTM!The command definition is well-structured with comprehensive usage documentation, clear examples for each swap type, and appropriate flags. The descriptions effectively communicate the currency constraints for each swap type.
100-207: LGTM!The handler implementation is well-structured with:
- Clear swap type parsing with convenient aliases (
sub,rev)- Proper currency defaults/constraints per swap type
- Robust input validation for send/receive amounts
- Clean output formatting with fee breakdown and limits
docs/grpc.md (2)
87-94: LGTM!The GetSwapQuote method documentation is clear and properly links to the request/response message types.
1037-1068: LGTM!The message documentation for
GetSwapQuoteRequestandGetSwapQuoteResponseis complete and accurately describes all fields. The request uses a oneof pattern for send_amount/receive_amount, and the response includes the fee breakdown with pair_info for limits.
| NetworkFee uint64 | ||
| } | ||
|
|
||
| func CalculateSwapQuote(swapType boltz.SwapType, sendAmount, receiveAmount uint64, fees *boltzrpc.SwapFees) (*SwapQuote, error) { |
There was a problem hiding this comment.
There is lots of options where calculations here can under-, overflow or divide by 0 if the input is nonsensical or the backend is malicious. Especially division by 0 is dangerous because that one panics. Instead of trying to catch them all by hand, we should use something like https://pkg.go.dev/go.dw1.io/safemath
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
internal/utils/fees.go (2)
41-52: Add default case for unhandled swap types.Both switch statements lack a default case. If an unknown
SwapTypeis passed, the function returns a quote with zeroBoltzFeeand zeroReceiveAmount/SendAmountwithout error, leading to silent failures.♻️ Suggested fix
if sendAmount > 0 { quote.SendAmount = sendAmount switch swapType { case boltz.NormalSwap: // Submarine: service fee on receive, so receive = (send - minerFee) / (1 + rate) rate := 1 + percentage.Ratio() receive := max(0, float64(sendAmount)-float64(minerFee)) / rate quote.ReceiveAmount = uint64(receive) quote.BoltzFee = percentage.Calculate(quote.ReceiveAmount) case boltz.ReverseSwap, boltz.ChainSwap: // Reverse/Chain: service fee on send quote.BoltzFee = percentage.Calculate(sendAmount) quote.ReceiveAmount = uint64(max(0, int64(sendAmount)-int64(quote.BoltzFee)-int64(minerFee))) + default: + return nil, fmt.Errorf("unsupported swap type: %s", swapType) }Apply the same pattern to the second switch (lines 56-70).
63-66: Consider checking for non-positive rate.The check only handles
rate == 0, but ifpercentage.Ratio() > 1(i.e., fee percentage > 100%),ratewould be negative, causingsendAmountto become negative before being cast touint64.♻️ Suggested fix
case boltz.ReverseSwap, boltz.ChainSwap: // Reverse/Chain: service fee on send, so send = (receive + minerFee) / (1 - rate) rate := 1 - percentage.Ratio() - if rate == 0 { - return nil, fmt.Errorf("division by zero") + if rate <= 0 { + return nil, fmt.Errorf("invalid fee percentage: rate must be positive") }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
internal/utils/fees.go
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-26T09:32:25.533Z
Learnt from: jackstar12
Repo: BoltzExchange/boltz-client PR: 444
File: internal/rpcserver/router.go:2036-2052
Timestamp: 2025-05-26T09:32:25.533Z
Learning: In boltz-client, when handling Boltz backend connectivity during startup, the code intentionally uses different error handling strategies: logger.Fatalf for version incompatibility (fail fast approach) and retry logic for availability issues. Version incompatibility requires immediate human intervention and should not be retried.
Applied to files:
internal/utils/fees.go
🧬 Code graph analysis (1)
internal/utils/fees.go (2)
pkg/boltzrpc/boltzrpc.pb.go (10)
SwapType(178-178)SwapType(210-212)SwapType(214-216)SwapType(223-225)SwapFees(2063-2070)SwapFees(2085-2085)SwapFees(2100-2102)MinerFees(2181-2188)MinerFees(2203-2203)MinerFees(2218-2220)pkg/boltz/api.go (2)
Percentage(79-79)NormalSwap(29-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI (ubuntu-latest, 1.24.x)
🔇 Additional comments (3)
internal/utils/fees.go (3)
16-21: LGTM!Clean struct definition that clearly encapsulates the quote components.
31-36: LGTM!Input validation correctly ensures mutually exclusive amount specification.
38-52: Calculation logic is correct.The fee calculation correctly handles:
- Submarine swaps: fee applied on receive amount with formula
receive = (send - minerFee) / (1 + rate)- Reverse/Chain swaps: fee applied on send amount
The use of
max(0, ...)appropriately prevents negative intermediate values.
Summary by CodeRabbit
New Features
API
Permissions
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.