looprpc: populate static address swap timing and cost fields#1150
Conversation
hieblmi
commented
May 29, 2026
- Added initiation_time, last_update_time, cost_server, cost_onchain, and cost_offchain to StaticAddressLoopInSwap.
- Populated static loop-in list responses with stored initiation/update timestamps.
- Reported the persisted quoted swap fee as cost_server once the static loop-in invoice has been paid.
- Left on-chain/off-chain costs at zero because static timeout-path miner fees are not currently persisted.
- Added focused test coverage for static loop-in cost reporting behavior.
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 static address loop-in functionality by exposing timing and cost information through the RPC interface. By updating the protobuf definitions and implementing logic to calculate and report these metrics, users can now gain better visibility into the lifecycle and financial performance of their static address swaps. Highlights
New Features🧠 You can now enable Memory (public preview) 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 the 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 counterproductive. 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
|
There was a problem hiding this comment.
Code Review
This pull request adds initiation time, last update time, and cost fields (server, on-chain, off-chain) to the StaticAddressLoopInSwap protobuf message and populates them in the server implementation. It also introduces a helper function to calculate these costs and adds corresponding unit tests. The review feedback correctly identifies a potential issue where calling UnixNano() on uninitialized time.Time fields can lead to unexpected negative timestamps, and suggests adding safety checks for zero-time values and nil pointers.
7aae14b to
1e5911c
Compare
|
@claude review this |
Code Review: looprpc: populate static address swap timing and cost fieldsOverviewThis PR adds five new fields to FindingsMinor issues1. Nil guard may silently hide a bug ( if swp == nil {
continue
}
if swp == nil {
log.Warnf("ListStaticAddressSwaps: nil swap in results, skipping")
continue
}If nil entries are impossible by contract (likely), the guard can be removed entirely. 2. Proto docs for timestamps don't mention units
int64 initiation_time = 7;
int64 last_update_time = 8;These are Unix nanoseconds (via 3. The function comment in // On-chain transaction cost.
int64 cost_onchain = 10;
// Off-chain routing fees.
int64 cost_offchain = 11;A reader calling What looks good
SummaryThe implementation is correct and safe. The three points above are all minor: the nil guard is cosmetic, and the two proto documentation gaps are quality-of-life issues for external consumers. Nothing here blocks merging, but items 2 and 3 would improve the API's self-documenting quality. |
|
Addressed Claude's minor feedback in
|
ecf01e2 to
8fe21dc
Compare
8fe21dc to
c6b4362
Compare
|
But it also includes pending swaps. I fixed it in one commit. Also I added a test for RPC in another commit. |
starius
left a comment
There was a problem hiding this comment.
LGTM! 💾
I found two issues and fixed them in separate commits.