-
Notifications
You must be signed in to change notification settings - Fork 38.7k
[0.13.2] wallet/rpc backports #9347
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
Conversation
Github-Pull: bitcoin#9295 Rebased-From: 1a6eacb
Github-Pull: bitcoin#9295 Rebased-From: c24a4f5
8ecc6a5 to
7270e1a
Compare
Github-Pull: bitcoin#9326 Rebased-From: bae1eef b05b1af
322e48c to
1b51e70
Compare
This resolves an issue where a wallet transaction which failed to relay previously because it couldn't make it into the mempool will not try again until restart, even though mempool conditions may have changed. Abandoned and known-conflicted transactions are skipped. Some concern was expressed that there may be users with many unknown conflicts would waste a lot of CPU time trying to add them to their memory pools over and over again. But I am doubtful these users exist in any number, if they do exist they have worse problems, and they can mitigate any performance issue this might have by abandoning the transactions in question. Github-Pull: bitcoin#9290 Rebased-From: f692fce
Github-Pull: bitcoin#9302 Rebased-From: b3a7410
1b51e70 to
f5d606e
Compare
| if (GetDepthInMainChain() == 0 && !isAbandoned() && InMempool()) { | ||
| CValidationState state; | ||
| /* GetDepthInMainChain already catches known conflicts. */ | ||
| if (InMempool() || AcceptToMemoryPool(false, maxTxFee, state)) { |
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.
Not sure if I solved this merge conflict correctly. fLimitFree should probably be true to backport the behavior how it is in master, otoh the other places in wallet.cpp don't apply the limit, so it should not be applied here for consistency either?
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.
My view point is this doesn't matter. I think its fine the way you did it, and more consistent with the rest of 0.13 code. But I also think it would be no big deal if it was rate limited. It happens every 20 mins anyway..
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.
I believe this should be false like in the other cases of the wallet behavior. The "free limiter" this this flag controls there is really a p2p network feature, and shouldn't really apply to wallet transactions... and in 0.13.2 we won't have removed the relaying of non-fee transactions yet.
|
Thanks for the backport, commit-list ACK f5d606e. I cannot answer your question about |
Github-Pull: bitcoin#9292 Rebased-From: 80d073c
Github-Pull: bitcoin#9322 Rebased-From: fa615d3
|
ACK 49a612f |
49a612f [qa] Don't set unknown rpcserialversion (MarcoFalke) c365556 Complain when unknown rpcserialversion is specified (Pieter Wuille) f5d606e Return txid even if ATMP fails for new transaction (Pieter Wuille) 35174a0 Make RelayWalletTransaction attempt to AcceptToMemoryPool. (Gregory Maxwell) a0f7ece Update for OpenSSL 1.1 API (Gregory Maxwell) 43bcfca [Wallet] Bugfix: FRT: don't terminate when keypool is empty (Jonas Schnelli) 0cc07f8 [QA] add fundrawtransaction test on a locked wallet with empty keypool (Jonas Schnelli)
Remaining wallet backports for 0.13.2 release
Edit: Sorry for force pushing a few times. There were two conflicts, so would be nice if someone could look over the cherry-picks.