Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Wallet] AddToWallet split in AddToWallet and LoadToWallet #1717

Merged
merged 4 commits into from
Jul 11, 2020
Merged

[Wallet] AddToWallet split in AddToWallet and LoadToWallet #1717

merged 4 commits into from
Jul 11, 2020

Conversation

furszy
Copy link

@furszy furszy commented Jun 29, 2020

Commits details:

1- [No functional changes] Stop using CWalletTx::WriteToDisk, and use directly CWalletDB::WriteTx.

2- Performance improvement, preventing several calls to AvailableCoins in CreateTransaction.

3- [No functional changes] Split AddToWallet in AddToWallet and LoadToWallet (these two were already divided by a boolean method argument)

@furszy furszy self-assigned this Jun 29, 2020
This was referenced Jul 1, 2020
Merged
Merged
@random-zebra
Copy link

This is an extremely good PR, I'd like to have it in 4.2 if possible.
Also this led to the discovery of #1734, and it should be probably rebased on top of it (i.e. conflict detection must be in CWallet::LoadToWallet).

@random-zebra random-zebra added this to In Progress in perpetual updating PIVX Core to BTC Core via automation Jul 7, 2020
@random-zebra
Copy link

This should be rebased, now that #1734 has been merged.

@Fuzzbawls Fuzzbawls added this to the 4.2.0 milestone Jul 8, 2020
@furszy furszy force-pushed the 2020_05_wallet_addToWallet_LoadToWallet branch from 8ec6ed7 to c101d25 Compare July 8, 2020 23:39
@furszy
Copy link
Author

furszy commented Jul 8, 2020

rebased

@random-zebra
Copy link

This does not compile now. Probably because the rebase on master included the changes from #1713, which moved (to wallet_zerocoin.cpp) some call to AddToWallet, with the (now removed) fFromLoadWallet parameter.

@furszy furszy force-pushed the 2020_05_wallet_addToWallet_LoadToWallet branch from c101d25 to f117fa9 Compare July 9, 2020 13:38
@furszy
Copy link
Author

furszy commented Jul 9, 2020

done. ccache played with my system..

random-zebra
random-zebra previously approved these changes Jul 9, 2020
View reviewed changes
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK f117fa9

@random-zebra random-zebra dismissed their stale review July 9, 2020 21:12

Needs rebase

@furszy
Fix insanity of CWalletDB::WriteTx and CWalletTx::WriteToDisk
266b1a0 
Coming from btc@0fd599767d2dabf25ac8c3543cb586cfc4b9d816
@furszy
Prevent multiple calls to CWallet::AvailableCoins
49f4124 
Coming from btc@bb16c8894becfba8764b13d448ba6e7e7f1608c2
@furszy
Split CWallet::AddToWallet into AddToWallet and LoadToWallet.
c3779c5 
This removes the fFromLoadWallet flag in AddToWallet.  These were already
effectively two methods.

Coming from btc@00f09c920c2e8906d2260251be6d1d2fa1bbb29d
@furszy
Wallet::AddToWallet if walletdb is null, create one and write the tx.
aaf063f
@furszy furszy force-pushed the 2020_05_wallet_addToWallet_LoadToWallet branch from f117fa9 to aaf063f Compare July 9, 2020 21:57
@furszy
Copy link
Author

furszy commented Jul 9, 2020

rebased again.

@Fuzzbawls Fuzzbawls changed the title Wallet: AddToWallet split in AddToWallet and LoadToWallet [Wallet] AddToWallet split in AddToWallet and LoadToWallet Jul 10, 2020
random-zebra
random-zebra approved these changes Jul 10, 2020
View reviewed changes
Copy link

@random-zebra random-zebra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re utACK aaf063f

perpetual updating PIVX Core to BTC Core automation moved this from In Progress to Ready Jul 11, 2020
Fuzzbawls
Fuzzbawls approved these changes Jul 11, 2020
View reviewed changes
Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK aaf063f

@random-zebra random-zebra added the Needs Backport Placeholder tag for anything needing a backport to prior version branches label Jul 11, 2020
@random-zebra random-zebra merged commit f928674 into PIVX-Project:master Jul 11, 2020
perpetual updating PIVX Core to BTC Core automation moved this from Ready to Done Jul 11, 2020
Fuzzbawls pushed a commit to Fuzzbawls/PIVX that referenced this pull request Jul 22, 2020
@furszy @Fuzzbawls
Fix insanity of CWalletDB::WriteTx and CWalletTx::WriteToDisk
c0e9b13 
Coming from btc@0fd599767d2dabf25ac8c3543cb586cfc4b9d816

Github-Pull: PIVX-Project#1717
Rebased-From: 266b1a0
Fuzzbawls pushed a commit to Fuzzbawls/PIVX that referenced this pull request Jul 22, 2020
@furszy @Fuzzbawls
Prevent multiple calls to CWallet::AvailableCoins
9aacb38 
Coming from btc@bb16c8894becfba8764b13d448ba6e7e7f1608c2

Github-Pull: PIVX-Project#1717
Rebased-From: 49f4124
Fuzzbawls pushed a commit to Fuzzbawls/PIVX that referenced this pull request Jul 22, 2020
@furszy @Fuzzbawls
Split CWallet::AddToWallet into AddToWallet and LoadToWallet.
609407d 
This removes the fFromLoadWallet flag in AddToWallet.  These were already
effectively two methods.

Coming from btc@00f09c920c2e8906d2260251be6d1d2fa1bbb29d

Github-Pull: PIVX-Project#1717
Rebased-From: c3779c5
Fuzzbawls pushed a commit to Fuzzbawls/PIVX that referenced this pull request Jul 22, 2020
@furszy @Fuzzbawls
Wallet::AddToWallet if walletdb is null, create one and write the tx.
eaacd29 
Github-Pull: PIVX-Project#1717
Rebased-From: aaf063f
@Fuzzbawls Fuzzbawls mentioned this pull request Jul 22, 2020
Merged
furszy added a commit that referenced this pull request Jul 24, 2020
@furszy
Merge #1758: [Backport] 4.2.0 backports
034518f 
dd22385 [GUI] Update translations from transifex (Fuzzbawls)
919e060 Solving old, not loaded at startup, transactions notification issue. (furszy)
943d84b NU name words separated by underscore instead of white space. (furszy)
fc042f1 Fixing UpgradeIndex position issue. (furszy)
531673c Customize network upgrades activation height for regtest at the startup. (furszy)
1337685 Sapling activation height for regtest (furszy)
15b1052 Miner: Unused reserveKey cleanup (furszy)
566b6fc [GUI] Load persisted transaction filter/sort during start (V3) (Mrs-X)
5d22583 [GUI] Recognize key event for clearing console (Fuzzbawls)
77880e0 GUI: Settings console message moved to read only. (furszy)
6b63591 options model unused signals cleanup (furszy)
b9e3859 GUI: Fixing settings display lang initialization (furszy)
b434cbf Transaction primitive unused methods cleanup. (furszy)
7f9037f missing validation_zerocoin_legacy added to CMakeLists.txt (furszy)
22bfc12 Move-only: ATMP zerocoin check moved to its own legacy file. (furszy)
24d9ac4 Move-only: DataBaseAccChecksum to zerocoin validation legacy. (furszy)
fb2993d Move-only: Move Zerocoin tx disconnection from DisconnectBlock to its own legacy file method. (furszy)
eaacd29 Wallet::AddToWallet if walletdb is null, create one and write the tx. (furszy)
609407d Split CWallet::AddToWallet into AddToWallet and LoadToWallet. (furszy)
9aacb38 Prevent multiple calls to CWallet::AvailableCoins (furszy)
c0e9b13 Fix insanity of CWalletDB::WriteTx and CWalletTx::WriteToDisk (furszy)

Pull request description:

  backports the following PRs that have been merged since the `4.2` branch-off:

  #1717
  #1733
  #1736
  #1742
  #1741
  #1744
  #1287
  #1751
  #1747
  #1749
  #1763

ACKs for top commit:
  furszy:
    utACK dd22385 , ready for 4.2 :) .

Tree-SHA512: 38dd47320b8a7de77879240c36dec7588671fc09bc7d4f0be573fb6a33d6d243d4adbf4451bd4b4da9c046fab058c1cb0eb2ec2d97052b4d1171ac959ae5a71f
random-zebra added a commit that referenced this pull request Aug 5, 2020
@random-zebra
Merge #1721: [Validation] Reduce cs_main locks during ConnectTip/Sync…
8e19562 
…WithWallets

9d1beb3 Move wallet callbacks into cs_main. (furszy)
4b4858b Remove SyncWithWallets wrapper function (Matt Corallo)
3b2807f Performance Regression Fix: Pre-Allocate txChanged vector (furszy)
1fe0be6 trivial: remove unused variable (Daniel Kraft)
f1eb073 Reduce cs_main locks during ConnectTip/SyncWithWallets (furszy)
5e155b4 Remove unused pwalletdb from CommitTransaction (furszy)
d8cc80a Remove CWalletDB* parameter from CWallet::AddToWallet (furszy)

Pull request description:

  Work done on top of #1717 .

  Essentially an improvement over `SyncWithWallets` & `SyncTransaction` flow to run without locking `cs_main`.
  (made as less modifications as possible over the zpiv wallet/chain files, no real need to improve that code.. most of it will disappear soon)

  It's an adapted version of bitcoin#7946.

ACKs for top commit:
  random-zebra:
    utACK 9d1beb3
  Fuzzbawls:
    ACK 9d1beb3

Tree-SHA512: 8fc494a1d94a7fdf048fd6d08eb3f831693c9cf252a871cee83405777f40ff36e2e9455efa66ed78d3d56e528336179de2ed54b8675cf819a00d8dafa19e909a
@Fuzzbawls Fuzzbawls removed the Needs Backport Placeholder tag for anything needing a backport to prior version branches label Aug 5, 2020
random-zebra added a commit that referenced this pull request Aug 18, 2020
@random-zebra
Merge #1725: [Backport] mempool score index.
e59d8e5 
6c566bb Add TxPriority class and comparator (Alex Morcos)
5e9df89 Make accessing mempool parents and children public (Alex Morcos)
1279a9f Add a score index to the mempool (furszy)
e5a72fe Store the total sig op count of a tx (furszy)

Pull request description:

  On top of #1707 and #1717 .

  Back ports bitcoin#6898 (without back porting the last commit as we need some cleanup on the miner area first).

ACKs for top commit:
  random-zebra:
    ACK 6c566bb
  Fuzzbawls:
    utACK 6c566bb

Tree-SHA512: c6fa788c59ea6e024e340e926e81462d0817617003034eecf9c3eeda3dfd5fbd2a7b6ec1ada3b867dba98079a195931f8a72fc2250207f475d8f44060452825b
@furszy furszy deleted the 2020_05_wallet_addToWallet_LoadToWallet branch November 29, 2022 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Fuzzbawls Fuzzbawls Fuzzbawls approved these changes

@random-zebra random-zebra random-zebra approved these changes

Assignees

@furszy furszy

Labels
Refactoring Upstream Wallet
Projects
perpetual updating PIVX Core to BTC Core
  
Done
Milestone
4.2.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@furszy @random-zebra @Fuzzbawls