Only fetch onchain transactions from LND within start_time and end_time range #202
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nordbjorn
force-pushed
the
feat/onchain-pagination-temp
branch
from
e0d6aac to
74518d2
Compare
matheusavi
reviewed
Jan 15, 2025
guggero
reviewed
Jan 16, 2025
nordbjorn
force-pushed
the
feat/onchain-pagination-temp
branch
from
a01826c to
901eff5
Compare
guggero
approved these changes
Jan 17, 2025
nordbjorn
force-pushed
the
feat/onchain-pagination-temp
branch
from
901eff5 to
771d1ae
Compare
bhandras
approved these changes
Jan 21, 2025
| // certain range, we will apply a buffer on the time range which we use to | ||
| // find start and end block heights. This should ensure we widen the block | ||
| // height range enough to fetch all relevant transactions within a time range. | ||
| const blockTimeRangeBuffer = time.Hour * 24 |
There was a problem hiding this comment.
While 24 hours seems like a safe bet, in practice we could probably lower this to 12 hours.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ensure that the
NodeAuditRPC makes use of the providedstart_timeandend_timearguments, in the sense that Faraday will only query the LND backend for onchain transactions using a block height range which corresponds to thestart_timeandend_time(with a buffer).Motivation and Context
The
NodeAuditrequest can currently fail for old LND nodes, with a lot of historical onchain activity. Even if the caller provides a small time range to produce the report for. This is because currently all onchain transactions are fetched from the LND backend, and then filtered in memory based on the given time range.This PR aims to alleviate most of the problem by ensuring Faraday does not need to query the entire history of onchain transactions when the caller provides a time range for the report.
Related issue
#177
Implementation Notes
I did not find any obvious way to resolve a timestamp to a block height, which needs to be provided for the queries to the LND backend. For now I chose an approach where we do a binary search on the entire blockchain to find the block height just before the provided timestamp.
This means we need to look up several block headers for each timestamp we want to resolve. I think this approach is fine for now, and in this context, considering that this RPC is a slow and heavy one anyways. Worst case is that we need to fetch ~20 block headers per timestamp we look up.
Future improvements
If we want to optimize the timestamp to block height resolving more, we could make use of this public API endpoint: https://mempool.space/docs/api/rest#get-block-timestamp
This endpoint seems to do a lookup from an internal DB (see here), which is likely more performant. In that case the current binary search mechanism could serve as only a fallback for if the mempool.space API is unavailable
We can also make use of pagination when querying onchain activity from the LND backend. This seems to require changes in the lndclient repository as well, to ensure that the
GetTransactionsRPC can be called with aindex_offsetandmax_transactions, like described herePull Request Checklist
MinLndVersionif your PR uses new RPC methods or fields oflnd.