Skip to content

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Dec 14, 2016

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.

@maflcko maflcko force-pushed the Mf1612-013back branch 2 times, most recently from 322e48c to 1b51e70 Compare December 14, 2016 12:51
gmaxwell and others added 2 commits December 14, 2016 13:55
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
if (GetDepthInMainChain() == 0 && !isAbandoned() && InMempool()) {
CValidationState state;
/* GetDepthInMainChain already catches known conflicts. */
if (InMempool() || AcceptToMemoryPool(false, maxTxFee, state)) {
Copy link
Member Author

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?

Copy link
Contributor

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..

Copy link
Contributor

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.

@laanwj
Copy link
Member

laanwj commented Dec 15, 2016

Thanks for the backport, commit-list ACK f5d606e. I cannot answer your question about fLimitFree.

@maflcko maflcko changed the title [0.13.2] wallet backports [0.13.2] wallet/rpc backports Dec 15, 2016
@maflcko
Copy link
Member Author

maflcko commented Dec 15, 2016

Just verified that this fixes the OpenSSL issues on fedora rawhide as well:

7823286:

wallet/test/crypto_tests.cpp:45:20: error: aggregate 'EVP_CIPHER_CTX ctx' has incomplete type and cannot be defined
     EVP_CIPHER_CTX ctx;
                    ^~~

After fb987b3 and a0f7ece, all is fine.

@btcdrak
Copy link
Contributor

btcdrak commented Dec 19, 2016

ACK 49a612f

@laanwj laanwj merged commit 49a612f into bitcoin:0.13 Dec 19, 2016
laanwj added a commit that referenced this pull request Dec 19, 2016
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)
@maflcko maflcko deleted the Mf1612-013back branch December 19, 2016 11:45
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants