feat(chain)!: Add time_of_sync to SyncRequest and FullScanRequest WIP
#1566
+251
−216
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
LagginTimes
force-pushed
the
time_of_sync
branch
2 times, most recently
from
47d602e to
806b99b
Compare
LagginTimes
force-pushed
the
time_of_sync
branch
from
806b99b to
e47204a
Compare
evanlinjin
reviewed
Aug 22, 2024
evanlinjin
reviewed
Aug 22, 2024
Comment on lines
-616
to
-637
| pub fn update_last_seen_unconfirmed(&mut self, seen_at: u64) -> ChangeSet<A> { | ||
| let mut changeset = ChangeSet::default(); | ||
| let unanchored_txs: Vec<Txid> = self | ||
| .txs | ||
| .iter() | ||
| .filter_map( | ||
| |(&txid, (_, anchors))| { | ||
| if anchors.is_empty() { | ||
| Some(txid) | ||
| } else { | ||
| None | ||
| } | ||
| }, | ||
| ) | ||
| .collect(); | ||
|
|
||
| for txid in unanchored_txs { | ||
| changeset.merge(self.insert_seen_at(txid, seen_at)); | ||
| } | ||
| changeset | ||
| } | ||
|
|
There was a problem hiding this comment.
I think removing this method should really be a separate commit. However, I'm wondering if there is an argument to keep it.
There was a problem hiding this comment.
My 2 sats is if both wallet and non-wallet based app will use SyncRequest and FullScanRequest then this shouldn't be needed.
LagginTimes
force-pushed
the
time_of_sync
branch
from
e47204a to
1b2a840
Compare
LagginTimes
force-pushed
the
time_of_sync
branch
from
1b2a840 to
9b75eeb
Compare
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.
Implements #1550.
Description
Adds
time_of_syncas a member ofSyncRequestandFullSyncRequestso we can set thelast_seenfor unconfirmed transactions during the sync/scan process inbdk_electrum.Notes to the reviewers
This PR is a WIP because setting the
last_seenfor unconfirmed transactions during the sync/scan process is yet to be implemented forbdk_esplora. I pushed my WIP towards setting thelast_seenforbdk_esplorabut I'm unsure if it is correct, as I did not have enough time to confirm and write a test for it before my travel. Please feel free to drop that commit if the concept is incorrect.Changelog notice
update_last_seen_unconfirmed()fromTxGraph.time_of_syncas a member ofSyncRequest.time_of_syncas a member ofFullScanRequest.Checklists
All Submissions:
cargo fmtandcargo clippybefore committingNew Features: