-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Backport bitcoin perutxo #1701
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
Backport bitcoin perutxo #1701
Conversation
da5217f to
83945a6
Compare
|
Hmm, I got compilation errors in most of the Travis jobs. On my laptop it compiles just fine. This is most likely due to gcc version differences. Does anyone know which gcc version is used in Travis builds? @flare maybe? |
|
@codablock I'll check against gitian as well, Travis-CI throws this |
|
gitian confirms compilation error: |
|
Sidenote: gitian is using gcc-4.8.4 on Ubuntu Trusty, same as Bitcoin 0.15 - so the issue must be somewhere in the code :) |
|
Setting up gitian now, because even if I install gcc-4.8 locally it's still not reproducable |
83945a6 to
7e3e9e4
Compare
|
Ok, builds are passing now. It was not a difference in gcc version, but a difference in boost versions and the use of boost::unordered_map vs. std::unordered_map. At the same time, I skipped backporting bitcoin#10249 as I planned to have C++11 related changes as part of my other backportings. I added this PR now, which changes boost::unordered_map to std::unordered_map, which also fixes the issue I had with compilation. |
|
I've spent some time yesterday trying to find the cause of this problem, but I was too late to report my findings. The problem is indeed with using #include <utility>
#include <tuple>
#include <boost/unordered_map.hpp>
class C {
int a_;
public:
explicit C(int &&a): a_(std::move(a)) {}
};
int main() {
boost::unordered_map<int, C> c_map;
c_map.emplace(std::piecewise_construct,
std::forward_as_tuple(1),
std::forward_as_tuple(2));
return 0;
}Compilation fails in both Ubuntu with GCC 4.8.4 and Fedora with GCC 7.2.1. Deep inside implementation of include <utility>
#include <tuple>
class C {
int a_;
public:
explicit C(int &&a): a_(std::move(a)) {}
};
int main() {
std::tuple<int&&, int&&> t = std::forward_as_tuple(1, 2);
C c(std::get<0>(t));
return 0;
}It's interesting to note that |
|
Although this particular problem is solved by using It's interesting to note that there is also no problem with emplacing using |
7e3e9e4 to
bff8b16
Compare
|
Now Travis isn't even starting the build with error "An error occurred while generating the build script." |
|
@codablock Travis is down —> https://www.traviscistatus.com |
|
Ah ok, next time I'll check that by myself :D About the failing tests I had after fixing the build issue: These were caused by an erroneous merge resolution I did in dash-tx.cpp. I removed too much code which was surrounded by SegWit changes. This is fixed now and will hopefully give green builds after Travis is up again. |
444f6b1 to
528d1ec
Compare
f478d98 Fix some empty vector references (Pieter Wuille) Tree-SHA512: a22022b9060cd39f8d349e8dd24490614c0028eae2fbc7186d0d26b1d47529049b2daee41f093c0722130274a0b6f7f8671c2295c8cb4a97028771eaff080734
5898279 scripted-diff: various renames for per-utxo consistency (Pieter Wuille) a5e02bc Increase travis unit test timeout (Pieter Wuille) 73de2c1 Rename CCoinsCacheEntry::coins to coin (Pieter Wuille) 119e552 Merge CCoinsViewCache's GetOutputFor and AccessCoin (Pieter Wuille) 580b023 [MOVEONLY] Move old CCoins class to txdb.cpp (Pieter Wuille) 8b25d2c Upgrade from per-tx database to per-txout (Pieter Wuille) b2af357 Reduce reserved memory space for flushing (Pieter Wuille) 41aa5b7 Pack Coin more tightly (Pieter Wuille) 97072d6 Remove unused CCoins methods (Pieter Wuille) ce23efa Extend coins_tests (Pieter Wuille) 5083079 Switch CCoinsView and chainstate db from per-txid to per-txout (Pieter Wuille) 4ec0d9e Refactor GetUTXOStats in preparation for per-COutPoint iteration (Pieter Wuille) 13870b5 Replace CCoins-based CTxMemPool::pruneSpent with isSpent (Pieter Wuille) 05293f3 Remove ModifyCoins/ModifyNewCoins (Pieter Wuille) 961e483 Switch tests from ModifyCoins to AddCoin/SpendCoin (Pieter Wuille) 8b3868c Switch CScriptCheck to use Coin instead of CCoins (Pieter Wuille) c87b957 Only pass things committed to by tx's witness hash to CScriptCheck (Matt Corallo) f68cdfe Switch from per-tx to per-txout CCoinsViewCache methods in some places (Pieter Wuille) 0003911 Introduce new per-txout CCoinsViewCache functions (Pieter Wuille) bd83111 Optimization: Coin&& to ApplyTxInUndo (Pieter Wuille) cb2c7fd Replace CTxInUndo with Coin (Pieter Wuille) 422634e Introduce Coin, a single unspent output (Pieter Wuille) 7d991b5 Store/allow tx metadata in all undo records (Pieter Wuille) c3aa0c1 Report on-disk size in gettxoutsetinfo (Pieter Wuille) d342424 Remove/ignore tx version in utxo and undo (Pieter Wuille) 7e00322 Add specialization of SipHash for 256 + 32 bit data (Pieter Wuille) e484652 Introduce CHashVerifier to hash read data (Pieter Wuille) f54580e error() in disconnect for disk corruption, not inconsistency (Pieter Wuille) e66dbde Add SizeEstimate to CDBBatch (Pieter Wuille) Tree-SHA512: ce1fb1e40c77d38915cd02189fab7a8b125c7f44d425c85579d872c3bede3a437760997907c99d7b3017ced1c2de54b2ac7223d99d83a6658fe5ef61edef1de3
…rsor() 3ff1fa8 Use override keyword on CCoinsView overrides (Russell Yanofsky) 24e44c3 Don't return stale data from CCoinsViewCache::Cursor() (Russell Yanofsky) Tree-SHA512: 08699dae0925ffb9c018f02612ac6b7eaf73ec331e2f4f934f1fe25a2ce120735fa38596926e924897c203f7470e99f0a99cf70d2ce31ff428b105e16583a861
…tweak 9417d7a Be much more agressive in AccessCoin docs. (Matt Corallo) f58349c Restore some assert semantics in sigop cost calculations (Matt Corallo) 3533fb4 Return a bool in SpendCoin to restore pre-per-utxo assert semantics (Matt Corallo) ec1271f Remove useless mapNextTx lookup in CTxMemPool::TrimToSize. (Matt Corallo) Tree-SHA512: 158a4bce063eac93e1d50709500a10a7cb1fb3271f10ed445d701852fce713e2bf0da3456088e530ab005f194ef4a2adf0c7cb23226b160cecb37a79561f29ca
…eCoin 5257698 Change semantics of HaveCoinInCache to match HaveCoin (Alex Morcos) Tree-SHA512: 397e9ba28646b81fffa53e55064735d4d242aaffdf8484506825f785b0e414f334e4c5cd1e4e1dd9a4b6d1f6954c7ecad15429934a1c4e8d39f596cbd9f5dd80
21180ff Simplify return values of GetCoin/HaveCoin(InCache) (Pieter Wuille) Tree-SHA512: eae0aa64fa1308191100cdc7cdc790c825f33b066c200a18b5895d7d5806cee1cc4caba1766ef3379a7cf93dde4bbae2bc9be92947935f5741f5c126d3ee991b
…n keypress 'q' 542ce6e Report [CANCELLED] instead of [DONE] when shut down during txdb upgrade (Jonas Schnelli) 83fbea3 Report txdb upgrade not more often then every 10% (Jonas Schnelli) 06c5b6e Show txdb upgrade progress in debug log (Jonas Schnelli) 316fcb5 Allow to cancel the txdb upgrade via splashscreen callback (Jonas Schnelli) ae09d45 Allow to shut down during txdb upgrade (Jonas Schnelli) 00cb69b [Qt] allow to execute a callback during splashscreen progress (Jonas Schnelli) Tree-SHA512: 23190f23f441bfd60821e49f8b3698a6bef97eb0e0ee659328e4a7395769ecd1616420eacc38aa1fa0ff62b9de5f13a0098dc798cdec6bff649575cefebc0db2
efeb273 Force on-the-fly compaction during pertxout upgrade (Pieter Wuille) Pull request description: It seems that LevelDB tends to leave the old "per txid" UTXO entries in the database lying around for a significant amount of time during and after the per-txout upgrade. This introduces a `CompactRange` function in the database wrapper, and invokes it after every batch of updates in `CCoinsViewDB::Upgrade()`. This lowers temporary disk usage during and after the upgrade. Tree-SHA512: fbf964c0a33f4e73709c999c8a2bfdef974779c15820907398a2f8828f5fa3e4e153ddd9031d6fc5083be81e22b999b9bd826fd063ad8b88f55c5e8342503290
…B compactions 8842d1a Add undocumented -forcecompactdb to force LevelDB compactions (Pieter Wuille) Pull request description: Tree-SHA512: de91f3f574f75248fa6e5091089c840957fae5a972ebcd2b89493f7d777d4658560a6f5a3b43ab0c9b2c333ad98f9f185ae224c9caffc1a5e8df369cc414f123
861f9a2 Skip remainder of init if upgrade is cancelled (Matt Corallo) Pull request description: Based on bitcoin#10919. Without this, if you cancel upgrade, you get a needless error: ERROR: VerifyDB(): *** irrecoverable inconsistency in block data at Tree-SHA512: aa47665682c6605ada376f1c100ce17cf8c4312427929eb2e75306f2199b47cbcdb4e0d98d5efcfefff03947b2c0fcbd3aab487a4ed14d50607df685c91a03d0
528d1ec to
9a450f7
Compare
|
Builds are finally green. After fixing the previous test failures, I got the next one which only happened in Travis but not locally. "make check" was failing with out-of-bounds access to a vector, without telling me which one it was. After some Travis debugging with some log outputs and increased BOOST_TEST_LOG_LEVEL, it turned out that some stream read with size 0 was performed on an empty vector, which is fine normally, but crashes with stl debugging enabled. Adding bitcoin#10250 to this PR fixed the issue. I'm repushing a clean version now (omitting the debug code), after this gives green Travis builds, we can finally merge. |
9a450f7 to
4cd1991
Compare
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.
Nice job, thanks! 👍
utACK
, bitcoin#25005, bitcoin#25410, bitcoin#24236, bitcoin#25438, bitcoin#25036, bitcoin#25489, bitcoin#25544, bitcoin#25594, bitcoin#13226, partial bitcoin#25218, bitcoin#26005 (wallet backports: part 6) 175970e merge bitcoin#13226: Optimize SelectCoinsBnB by tracking the selection by index rather than by position (Kittywhiskers Van Gogh) 27933a7 merge bitcoin#25594: Return BResult from restoreWallet (Kittywhiskers Van Gogh) b75f1ad partial bitcoin#26005: Fix error handling (copy_file failure in RestoreWallet, and in general via interfaces) (Kittywhiskers Van Gogh) 9f5845c partial bitcoin#25218: introduce generic 'Result' class and connect it to CreateTransaction and GetNewDestination (Kittywhiskers Van Gogh) db7e174 merge bitcoin#25544: don't iter twice when getting the cached debit/credit amount (Kittywhiskers Van Gogh) 48f5c10 merge bitcoin#25489: change `ScanForWalletTransactions` to use `Ticks(Dur2 d)` (Kittywhiskers Van Gogh) 4aafd98 merge bitcoin#25036: Save wallet scan progress (Kittywhiskers Van Gogh) e2f053a merge bitcoin#25438: remove unused methods in classes `CDBIterator,CDBWrapper,CCoinsViewDBCursor` (Kittywhiskers Van Gogh) 79fcd30 merge bitcoin#24236: Remove utxo db upgrade code (Kittywhiskers Van Gogh) d13ff52 fix: use URL quoting for password fields due to Base64 in pre-v0.12.2.x (Kittywhiskers Van Gogh) 7b6af94 fix: allow argument-free RPC calls with nodes running v0.12.2.x and lower (Kittywhiskers Van Gogh) 1026f11 fix: make `get_previous_release.py` translate triples for pre-v0.12.2.x (Kittywhiskers Van Gogh) a0b2c7d fix: make `get_previous_releases.py` understand windows target triples (Kittywhiskers Van Gogh) 51237c9 merge bitcoin#25410: fix warning: "argument name 'feerate' in comment does not match parameter name" (Kittywhiskers Van Gogh) 2438b9f merge bitcoin#25005: remove extra wtx lookup in 'AvailableCoins' + several code cleanups (Kittywhiskers Van Gogh) a498336 refactor: use initializer to set `nCoinType` instead of manual assignment (Kittywhiskers Van Gogh) ae24276 merge bitcoin#25083: Set effective_value when initializing a COutput (Kittywhiskers Van Gogh) a883a8e merge bitcoin#24666: Fix coinselection.h include, Make COutput a struct (Kittywhiskers Van Gogh) f447b5d merge bitcoin#24711: Postpone wallet loading notification for encrypted wallets (Kittywhiskers Van Gogh) 39557f9 fix: include `BOOST_AUTO_TEST_SUITE_END` within namespace `wallet` (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * When backporting [bitcoin#23497](bitcoin#23497) (see [dash#6732](#6732)), some `BOOST_AUTO_TEST_SUITE_END()`s were placed _outside_ the `wallet` namespace scope, which resulted in incorrect behavior when rebasing this pull request over [dash#6732](#6732). This has been resolved. * Backports of [bitcoin#25083](bitcoin#25083) and [bitcoin#25005](bitcoin#25005) do not include changes to `wallet/rpc/spend.cpp` as the `sendall()` RPC has not been implemented in `develop` (f453998) as of this writing. * Backporting [bitcoin#24236](bitcoin#24236) required a fair number of additional considerations, firstly because the pull request introduces a test, `feature_unsupported_utxo_db.py`, that requires the last-capable of version Dash Core to generate a legacy UTXO database (the current database format debuted in [dash#1701](#1701), included in v0.12.2.2) and secondly, because v0.15 is the earliest expected version of Dash Core to work smoothly with our test framework ([source](https://github.com/dashpay/dash/blob/f453998bb76dee8fe016138da664dc281fc7f1d3/test/functional/mempool_compatibility.py#L29)). As we require the last release cycle that generates the legacy UTXO database by default, we used the last version of the v0.12.1.x family, v0.12.1.5 as the old node used in `feature_unsupported_utxo_db.py`. This brings unique challenges which are elaborated on below. * The test framework relies on `generatetoaddress`, a wallet-independent RPC call that was introduced in [bitcoin#7671](bitcoin#7671) to replace the old wallet-dependent `generate` RPC. It was backported as 760d58e in [dash#1790](#1790) and debuted in v0.12.3. We cannot use the `self.generate()` helper as simply dispatches a `generatetoaddress` ([source](https://github.com/dashpay/dash/blob/f453998bb76dee8fe016138da664dc281fc7f1d3/test/functional/test_framework/test_node.py#L343-L345)), which does not exist in v0.12.1.5. We work around this by using the more archaic `self.nodes[0].rpc.generate()`, in line with usage in `rpc_generate.py` ([source](https://github.com/dashpay/dash/blob/f453998bb76dee8fe016138da664dc281fc7f1d3/test/functional/rpc_generate.py#L25-L26)). * Releases before v0.12.3.x did not include the target triples in their naming convention (see tag [`v0.12.3.1`](https://github.com/dashpay/dash/releases/tag/v0.12.3.1) vs [`v0.12.1.5`](https://github.com/dashpay/dash/releases/tag/v0.12.1.5)), which causes problems when fetching packages based on the target `HOST` of the build environment. This has been remedied by introducing awareness for `RPi2`, `osx` and `linux{32,64}` labels. * Note, `RPi2` corresponds to `arm-linux-gnueabihf` based on the Gitian descriptor that was removed in [dash#2183](#2183) ([source](https://github.com/dashpay/dash/blob/dac090964fbdd96d896f725932aada71db736abf/contrib/gitian-descriptors/gitian-rpi2.yml#L26)) * This was also taken as an opportunity to introduce awareness for the `win32` and `win64` labels which persist even now (see tag [`v22.1.2`](https://github.com/dashpay/dash/releases/tag/v22.1.2)) * Currently, when we want to make RPC calls without arguments, a blank object is sent to by the RPC dispatcher. This is acceptable as objects are supported since [bitcoin#8811](bitcoin#8811) (backported as eb7a6b0 in [dash#1851](#1858)), included in v0.12.3. Unfortunately, since we are dealing with a version of Dash that is ancient even by previous release standards, this prevents the dispatcher from even acknowledging the RPC server is active (see below). <details> <summary>Test failure:</summary> ``` $ ./test/functional/feature_unsupported_utxo_db.py 2025-06-24T15:06:53.642000Z TestFramework (INFO): PRNG seed is: 1995903613070947610 2025-06-24T15:06:53.642000Z TestFramework (INFO): Initializing test directory /tmp/dash_func_test_1tqzf43o 2025-06-24T15:06:53.642000Z TestFramework (INFO): Create previous version (v0.12.1.5) utxo db 2025-06-24T15:06:53.895000Z TestFramework (ERROR): JSONRPC error Traceback (most recent call last): File "/home/eclair/Repositories/dashpay/dash/test/functional/test_framework/test_framework.py", line 161, in main self.run_test() File "/home/eclair/Repositories/dashpay/dash/./test/functional/feature_unsupported_utxo_db.py", line 35, in run_test self.start_node(0) File "/home/eclair/Repositories/dashpay/dash/test/functional/test_framework/test_framework.py", line 647, in start_node node.wait_for_rpc_connection() File "/home/eclair/Repositories/dashpay/dash/test/functional/test_framework/test_node.py", line 276, in wait_for_rpc_connection rpc.getblockcount() File "/home/eclair/Repositories/dashpay/dash/test/functional/test_framework/coverage.py", line 49, in __call__ return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/eclair/Repositories/dashpay/dash/test/functional/test_framework/authproxy.py", line 144, in __call__ raise JSONRPCException(response['error'], status) test_framework.authproxy.JSONRPCException: Params must be an array (-32600) 2025-06-24T15:06:53.947000Z TestFramework (INFO): Stopping nodes [...] ``` </details> * This is worked around by dispatching an empty array instead of an empty object, it has the same practical effect (conveying the lack of arguments) and is forwards-compatible. * Currently, the password used for RPC authentication in Base16 encoded. This is a result of [dash#1454](#1454), which switched away from Base64. Unfortunately, this benefit only extends to v0.12.2.0 onwards, which is too new for the old node needed in our functional test. Worse still, it was not the URL-safe variant of Base64, so attempting to establish a connection with the old node resulted in sporadic failures (see below). <details> <summary>Test failure:</summary> ``` $ ./test/functional/feature_unsupported_utxo_db.py 2025-06-24T15:13:27.517000Z TestFramework (INFO): PRNG seed is: 3228606794541395700 2025-06-24T15:13:27.518000Z TestFramework (INFO): Initializing test directory /tmp/dash_func_test_97d987gf 2025-06-24T15:13:27.518000Z TestFramework (INFO): Create previous version (v0.12.1.5) utxo db 2025-06-24T15:13:27.769000Z TestFramework (ERROR): Unexpected exception caught during testing Traceback (most recent call last): File "/home/eclair/Repositories/dashpay/dash/test/functional/test_framework/test_framework.py", line 161, in main self.run_test() File "/home/eclair/Repositories/dashpay/dash/./test/functional/feature_unsupported_utxo_db.py", line 35, in run_test self.start_node(0) File "/home/eclair/Repositories/dashpay/dash/test/functional/test_framework/test_framework.py", line 647, in start_node node.wait_for_rpc_connection() File "/home/eclair/Repositories/dashpay/dash/test/functional/test_framework/test_node.py", line 270, in wait_for_rpc_connection rpc = get_rpc_proxy( ^^^^^^^^^^^^^^ File "/home/eclair/Repositories/dashpay/dash/test/functional/test_framework/util.py", line 341, in get_rpc_proxy proxy = AuthServiceProxy(url, **proxy_kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/eclair/Repositories/dashpay/dash/test/functional/test_framework/authproxy.py", line 78, in __init__ authpair = user + b':' + passwd ~~~~~^~~~~~ TypeError: unsupported operand type(s) for +: 'NoneType' and 'bytes' 2025-06-24T15:13:27.822000Z TestFramework (INFO): Stopping nodes [...] ``` </details> * This is worked around by escaping URL special characters for the password field. It is practically a no-op for the Base16 passwords generated from v0.12.2.0 onwards but allows connection and authentication with older versions of Dash Core. ## How Has This Been Tested? A full CoinJoin session run on 89cd497  ## Breaking Changes None expected. ## Checklist - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 175970e PastaPastaPasta: utACK 175970e Tree-SHA512: 6a6fe593e25cc5c99da41333a334655010cc387f8c3e390d472de29782aaa6aa67276e2e00a90609facabe4f1817fa0152e6b6765f97e07541fd378ef0f4d566
Continuation of #1691
This PR contains the actual per-utxo patch (bitcoin#10195) and fixes which came later in Bitcoin.
With this PR, Dash will do a one-time db-upgrade to the new schema on first start. While this happens, dash-qt will show the progress on the loading screen. This progress is currently badly positioned. If it's ok for you all, I'll fix this in a later PR.