-
Notifications
You must be signed in to change notification settings - Fork 420
fix(electrum): prevent fetch_prev_txout from querying coinbase transactions
#1756
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(electrum): prevent fetch_prev_txout from querying coinbase transactions
#1756
Conversation
|
Needs a test then this looks good to go for 1.0.0. |
5d3b2f5 to
2598e1d
Compare
2598e1d to
aeef579
Compare
b4f6939 to
4cf7254
Compare
|
For the test can't we also build a |
4cf7254 to
ccf13ec
Compare
|
tACK ccf13ec. Test breaks when coinbase TX queried. Failed test
|
Yes, something like this would work as well: I believe the current implementation in the PR is more straightforward and easier to understand. However, I would appreciate hearing others' thoughts on this. |
ValuedMammal
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK ccf13ec
|
@LagginTimes let's include both tests. The |
evanlinjin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK ccf13ec
notmandatory
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK ccf13ec
|
Looks like this is ready to go once |
ccf13ec to
1d2ebea
Compare
…sactions \`fetch_prev_txout\` should not try to query the prevouts of coinbase transactions, as it may result in the Electrum server returning an error which would cause the overall `sync` to fail.
1d2ebea to
d4ef266
Compare
|
|
evanlinjin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK d4ef266
Fixes #1698.
Description
fetch_prev_txoutshould not try to query the prevouts of coinbase transactions, as it may result in the Electrum server returning an error which would cause the overallsync/full_scanto fail. Without this critical bug fix,bdk_electrumwill crash when someone tracks an address being mined to.Notes to the reviewers
Changelog notice
fetch_prev_txoutno longer queries coinbase transactions.Checklists
All Submissions:
cargo fmtandcargo clippybefore committingBugfixes: