forwarding ability: introduce db helpers#240
Conversation
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 introduces several helper utilities and database access patterns to support the forwarding ability algorithm. By adding efficient data retrieval methods and a streaming iterator, the changes enable the system to process historical channel event data more effectively while maintaining memory efficiency through chunked pagination. 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
|
![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 a generic Quantile function for numeric types and expands the chanevents store with new retrieval methods, including a streaming iterator for channel events and utilities for mapping short channel IDs to peers. Review feedback identifies a compilation error in store.go due to a missing log import, highlights potential precision loss when calculating quantiles for integer types, and recommends explicitly listing columns in SQL queries instead of using SELECT * for better maintainability.
| log.Tracef("Iterating channel events for channel ID %d "+ | ||
| "between %v and %v", channelID, startTime, endTime) |
There was a problem hiding this comment.
The log.Tracef call will cause a compilation error because log is not defined or imported in this file. Please ensure the necessary logging package is imported and the log variable is initialized, similar to how it's done in other packages in this repository.
ViktorT-11
left a comment
ViktorT-11
left a comment
There was a problem hiding this comment.
Looks great, LGTM 🔥! Just leaving some suggestions below, non of which are required.
|
|
||
| // GetLatestChannelUpdateBefore returns the latest channel event before a given | ||
| // time (exclusive). If no event is found, it returns (nil, nil). | ||
| func (s *Store) GetLatestChannelUpdateBefore(ctx context.Context, |
There was a problem hiding this comment.
Could potentially make it optional to specify which event type this function uses (and then rename the function)?
There was a problem hiding this comment.
I skipped this as there's only a single caller right now.
Add GetLatestChannelUpdateBefore, which fetches the most recent EventTypeUpdate strictly before a given instant. Forwarding-ability analyses that summarise behaviour over a window must seed their state from the channel's balance at the window's lower bound; without the ability to look back past that bound, the first events in the window have no baseline to compare against. The lookup tolerates missing predecessors by returning (nil, nil), letting callers distinguish "no prior update" from a genuine error. Coverage exercises both the present and absent cases against the existing TestStore fixture.
Add ScidToPeerMap, which materialises a snapshot of every short channel id paired with the pubkey of the channel's remote peer. Forwarding-data sources index events by short channel id, but downstream analyses need to attribute behaviour to the peer, not the channel. The map skips channels whose short channel id is still zero (unconfirmed), so callers see only fully advertised channels. Coverage extends TestStore with a two-channel fixture pinning the join.
Add GetChannelByShortChanID, the inverse of AddChannel. The forwarding-ability analyzer receives scids from lnd's forwarding history and must map them back to the chanevents store's internal channel id to query events.
Add Quantile, a generic linear-interpolation q-quantile over a slice of sortable numeric values. The forwarding-ability analyzer needs to characterise the distribution of historical forwarded amounts and uses a configurable percentile as the headline statistic; lifting the computation into its own helper keeps the analyzer focused on forwarding logic and gives the quantile contract its own table-driven test that covers the interpolation rule and the empty/out-of-bounds error paths.
bitromortac
force-pushed
the
2604-fwd-prep-10
branch
from
42ef61c to
41ce984
Compare
bitromortac
merged commit 65ef4ab
into
lightninglabs:faraday-forwarding-ability
2 participants
Introduces helpers for the forwarding ability algorithm. A preview for how this is used can be seen here https://github.com/bitromortac/faraday/tree/2604-fwd-prep-11.