Skip to content

rebase expand-getblockstats#2

Closed
jnewbery wants to merge 2619 commits intoexpand-getblockstatsfrom
2019-06-rebase-expand-getblockstats
Closed

rebase expand-getblockstats#2
jnewbery wants to merge 2619 commits intoexpand-getblockstatsfrom
2019-06-rebase-expand-getblockstats

Conversation

@jnewbery
Copy link
Copy Markdown

@jnewbery jnewbery commented Jul 5, 2019

bitcoin#14802 improved the performance of the getblockstats RPC method by ~100x by using data from the undo files instead of txindex. It also removes the requirement to run txindex to generate certain statistics.

This PR rebases the expand-getblockstats patchset on master to take advantages of those performance improvements.

To review: verify that git diff 2019-06-rebase-expand-blockstats~6 2019-06-rebase-expand-blockstats achieves the same as git diff expand-blockstats~6 expand-getblockstats.

MarcoFalke and others added 30 commits May 14, 2019 09:16
f1a77b0 [docs] Add doxygen comment for CReserveKey (John Newbery)
37796b2 [docs] Add doxygen comment for CKeyPool (John Newbery)
ef2d515 [wallet] move-only: move CReserveKey to be next to CKeyPool (John Newbery)

Pull request description:

  Docs/move-only

  Adds doxygen comments for the CKeyPool and CReserveKey objects. The way these work is pretty confusing and it's easy to overlook details (eg bitcoin#15557 (comment)).

  These are on the verbose side, but I think too much commenting is better than not enough. Happy to take feedback on what's an appropriate level.

ACKs for commit f1a77b:
  jonatack:
    Thanks, John. Re-ACK f1a77b0, doc-only changes with respect to previous review.
  jb55:
    ACK f1a77b0

Tree-SHA512: 8bc97c7029cd2e8d9bfd2d2144eeff73474c71eda5a9d10817e1578ca0b70da677252037d83143faaff1808e2193408a21a8a89d36049eac77fd313990f0b67b
2d5cf4c rpc: Speedup getrawmempool when verbose=true (João Barbosa)

Pull request description:

  Instead of calling `pushKV(hash, info)`, which incurs in duplicate key checks, call `__pushKV` which (currently) doesn't.

  Improves RPC `getrawmempool` and REST `/rest/mempool/contents.json`.

  Fixes bitcoin#14765.

ACKs for commit 2d5cf4:

Tree-SHA512: c3e91371bb41f39e79dcef820815e1dc27fb689ca3c4bf3a00467d2215b3baecd44d9792f7a481577a5b7ae1fc6cbaa07b1cd62123b845082eba65b35c2b3ca5
The original ORCHID prefix was deprecated as of 2014-03, the new
ORCHIDv2 prefix was allocated by RFC7343 as of 2014-07. We did not
consider the original ORCHID prefix routable, and I don't see any reason
to consider the new one to be either.
fad0ce5 tests: Fail if RPC has been added without tests (MarcoFalke)

Pull request description:

  Need to be run with --coverage

ACKs for commit fad0ce:
  ryanofsky:
    utACK fad0ce5. New comment in travis.yml is the only change since last review.

Tree-SHA512: b53632dfe9865ec06991bfcba2fd67238bebbb866b355f09624eaf233257b2bca902caac6c24abb358b2f4c1c43f28ca75e30982765911e1a117102df65276d9
faf38bc build with -fstack-reuse=none (MarcoFalke)

Pull request description:

  All versions of gcc that we commonly use for building are subject to bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90348. To work around that, set `-fstack-reuse=none` for all builds. See bitcoin#15959 (comment)

  This option does not exist for clang https://clang.llvm.org/docs/ClangCommandLineReference.html#target-independent-compilation-options, but it does for gcc https://gcc.gnu.org/onlinedocs/gcc/Code-Gen-Options.html

ACKs for commit faf38b:

Tree-SHA512: f5597583402d31ea7b89ac358a6f4a537bbb82a1dde2bd41bac65ee0fd88c7af75ee4c24c6b8eed6b139bf4dbf07ab2987f8fc30fb3eab179cd2d753a8a2a47d
1b05dff Fix portability issue with pthreads (grim-trigger)

Pull request description:

  This change resolves the following issue:
  bitcoin#15951

  Only tested on OpenBSD 6.5/amd64

ACKs for commit 1b05df:
  fanquake:
    tACK 1b05dff. Tested on OpenBSD6.4 (`vagrant`).
  laanwj:
    utACK 1b05dff

Tree-SHA512: af48581af32820d5adc9ae5abb44f8f1b592c323f86fe2484108b81629389f6ef347598f9a087aa6476ac553e59828cd7927bb4ab11dc70e7c9a944a92fc54ae
along with DisconnectResult, and CBlockIndexWorkComparator.

The CChainState interface needs to be known to the rest of the system because
many global functions will move to CChainState methods. This is to allow
other parts of the system to be parameterized per chainstate instance
instead of assuming a single global.
To be used once we move global functions (e.g. FlushStateToDisk()) into
CChainState methods.

Thanks to Marco Falke for suggestions
Also renames global methods for clarity:

- ::FlushStateToDisk() -> CChainState::ForceFlushStateToDisk()
  - This performs an unconditional flush.

- ::PruneAndFlush() -> CChainState::PruneAndFlush()
We introduce CChainState.m_cached_finished_ibd because the static state it
replaces would've been shared across all CChainState instances.
c01c065 Do not construct out-of-bound pointers in SHA512/SHA1/RIPEMD160 code (Pieter Wuille)

Pull request description:

  This looks like an issue in the current SHA256/512 code, where a pointer outside of the area pointed to may be constructed (this is UB in theory, though in practice every supported platform treats pointers as integers).

  I discovered this while investigating bitcoin#14580. Sadly, it does not fix it.

ACKs for commit c01c06:
  practicalswift:
    utACK c01c065

Tree-SHA512: 47660e00f164f38c36a1ab46e52dd91cd33cfda6a6048d67541c2f8e73c050d4d9d81b5c149bfad281212d52f204f57bebf5b19879dc7a6a5f48aa823fbc2c02
The description reads:

```
# This script runs all contrib/devtools/lint-*.sh files, and fails if any exit
# with a non-zero status code.
```

This runs all scripts and returns with a non-zero exit code if any failed.
f3b90f2 Run all lint scripts (Julian Fleischer)

Pull request description:

  The description reads:

  ```
  # This script runs all contrib/devtools/lint-*.sh files, and fails if any exit
  # with a non-zero status code.
  ```

  This runs all scripts and returns with a non-zero exit code if any failed.

ACKs for commit f3b90f:

Tree-SHA512: 4f1f6435855dd5074a38c5887be6f096ec66f4dbe8712bdfd5fed0c510f1b2c083b7318cf3bfbdcc85982429fb7b4309e57ce48cc11736c86376658ec7ffea8f
…ly been pruned

fa7e311 [doc] rpcwallet: Only fail rescan when blocks have been pruned (MarcoFalke)
aaaa57c scripted-diff: Bump copyright headers in wallet (MarcoFalke)
faf3729 wallet: Only fail rescan when blocks have actually been pruned (MarcoFalke)

Pull request description:

  This brings the behaviour of the import* calls closer to importmulti. After this change, the difference between importmulti and the other import* calls is

  * that in importmulti you can "opt-out" of scanning early blocks by setting a later timestamp.
  * that in importmulti the wallet will successfully import the data, but fail to rescan. Whereas in the other calls, the wallet will abort before importing the data.

ACKs for commit fa7e31:
  promag:
    utACK fa7e311.
  jnewbery:
    utACK fa7e311

Tree-SHA512: a57d52ffea94b64e0eb9b5d3a7a63031325833908297dd14eb0c5251ffea3b2113b131003f1db4e9599e014369165a57f107a7150bb65e4c791e5fe742f33cb8
662d117 Add option to create an encrypted wallet (Andrew Chow)

Pull request description:

  This PR adds a new `passphrase` argument to `createwallet` which will create a wallet that is encrypted with that passphrase.

  This is built on bitcoin#15226 because it needs to first create an empty wallet, then encrypt the empty wallet and generate new keys that have only been stored in an encrypted state.

ACKs for commit 662d11:
  laanwj:
    utACK 662d117
  jnewbery:
    Looks great. utACK 662d117

Tree-SHA512: a53fc9a0f341eaec1614eb69abcf2d48eb4394bc89041ab69bfc05a63436ed37c65ad586c07fd37dc258ac7c7d5e4f7f93b4191407f5824bbf063b4c50894c4a
fa8ced3 doc: Mention blocksonly in reduce-traffic.md, unhide option (MarcoFalke)
fa320de test: Add test for p2p_blocksonly (MarcoFalke)
fa3872e test: Format predicate source as multiline on error (MarcoFalke)
fa1dce7 net: Rename ::fRelayTxes to ::g_relay_txes (MarcoFalke)

Pull request description:

  This is de-facto no longer hidden

ACKs for commit fa8ced:
  jamesob:
    utACK bitcoin@fa8ced3

Tree-SHA512: 474fbdee6cbd035ed9068a066b6056c1f909ec7520be0417820fcd1672ab3069b53f55c5147968978d9258fd3a3933fe1a9ef8e4f6e14fb6ebbd79701a0a1245
…ate key

8794a4b QA: add test for HKDF HMAC_SHA256 L32 (Jonas Schnelli)
551d489 Add HKDF HMAC_SHA256 L=32 implementations (Jonas Schnelli)
3b64f85 QA: add test for CKey::Negate() (Jonas Schnelli)
463921b CKey: add method to negate the key (Jonas Schnelli)

Pull request description:

  This adds a limited implementation of `HKDF` (defined by rfc5869) that supports only HMAC-SHA256  and length output of 32 bytes (will be required for v2 transport protocol).

  This PR also includes a method to negate a private key which is useful to enforce public keys starting with 0x02 (or 0x03) (a requirement for the v2 transport protocol). The new `CKey::Negate()` method is pretty much a wrapper around `secp256k1_ec_privkey_negate()`.

  Including tests.

  This is a subset of bitcoin#14032 and a pre-requirement for the v2 transport protocol.

ACKs for commit 8794a4:

Tree-SHA512: 5341929dfa29f5da766ec3612784baec6a3ad69972f08b5a985a8aafdae4dae36f104a2b888d1f5d1f33561456bd111f960d7e32c2cc4fd18e48358468f26c1a
01971da docs: Add productivity notes for "dummy rebases" (Carl Dong)

Pull request description:

  When rebasing, we often want to do a "dummy rebase" whereby we are not rebasing over an updated master. This is because rebases can be confusing enough already, and we don't want to resolve upstream conflicts together with our local rebase conflicts due to fixup commits, commit rearrangements, and such. This productivity section details how to do such "dummy rebase"s.

ACKs for commit 01971d:

Tree-SHA512: 241a451cec01dc9a01a2286bdee1441cac6d28007f5b173345744d2abf436da916c3f2553ff0d1c5b3687055107b37872dda9529288645e4bae7b3cb46923b7e
… HAVE_THREAD_LOCAL

41ab2a8 fix static_assert for macro HAVE_THREAD_LOCAL (orient)

Pull request description:

  Utils and libraries: fix static_assert for macro HAVE_THREAD_LOCAL if define DEBUG_LOCKCONTENTION

ACKs for commit 41ab2a:
  laanwj:
    utACK 41ab2a8

Tree-SHA512: 6c1d20375a70cbdef1140c544f443106d6bf6c34b1da2ddc66739f2b662a0d6b800288f48bf451a3d5937bac7e40b8ecda3a4effcc978d0093fc497410447ea7
Also explain the param in all RPCs
…Locator(). Remove assumeLocked().

9402ef0 Remove temporary method assumeLocked(). Remove LockingStateImpl. Remove redundant cs_main locks. (practicalswift)
593a8e8 wallet: Use chain.lock() instead of temporary chain.assumeLocked() (practicalswift)

Pull request description:

  Fixes bitcoin#16028.

  Problem description:

  `LockAnnotation lock(::cs_main)` is a guarantee to the compiler thread analysis that `::cs_main` is locked (when it couldn't be determined otherwise).

  Despite being annotated with the locking guarantee ...

  https://github.com/bitcoin/bitcoin/blob/65526fc8666fef35ef908dbc225f706bef642c7e/src/interfaces/chain.cpp#L134-L138

  ... `getTipLocator()` reads `chainActive` (via `::ChainActive()`) without holding `cs_main`.

  This can be verified by adding the following `AssertLockHeld(cs_main)`:

  ```
  $ git diff
  diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp
  index 5962328..9fc693a0f 100644
  --- a/src/interfaces/chain.cpp
  +++ b/src/interfaces/chain.cpp
  @@ -134,6 +134,7 @@ class LockImpl : public Chain::Lock
       CBlockLocator getTipLocator() override
       {
           LockAnnotation lock(::cs_main);
  +        AssertLockHeld(::cs_main);
           return ::ChainActive().GetLocator();
       }
       Optional<int> findLocatorFork(const CBlockLocator& locator) override
  $ make check
  ../build-aux/test-driver: line 107: 12881 Aborted                 "$@" > $log_file 2>&1
  FAIL: qt/test/test_bitcoin-qt
  ```

ACKs for commit 9402ef:
  MarcoFalke:
    utACK 9402ef0
  ryanofsky:
    utACK 9402ef0. Changes are consolidating commits and removing redundant lock2 cs_main calls

Tree-SHA512: 0a030bf0c07eb53194ecc246f973ef389dd42a0979f51932bf94bdf7e90c52473ae03be49718ee1629582b05dd8e0dc020b5a210318c93378ea4ace90c0f9f72
NicolasDorier and others added 26 commits June 20, 2019 12:45
…n instead of two

Instead of creating a redeemScript with CreateMultisigRedeemscript and
then getting the destination with AddAndGetDestinationForScript, do
both in the same function.

CreateMultisigRedeemscript is changed to AddAndGetMultisigDestination.
It creates the redeemScript and returns it via an output parameter. Then
it calls AddAndGetDestinationForScript to add the destination to the
keystore and get the proper destination.

This allows us to inspect the public keys in the redeemScript before creating
the destination so that the correct destination is used when uncompressed
pubkeys are in the multisig.
Qt docs: This attribute must be set before QGuiApplication is 
constructed.
fa00326 ci: Run extended tests (MarcoFalke)

Pull request description:

  Currently not run in this repo, but in https://cirrus-ci.com/github/MarcoFalke/bitcoin

  https://cirrus-ci.com/github/MarcoFalke/bitcoin/1903-ciExt

ACKs for commit fa0032:

Tree-SHA512: e74b21084fe88dacedc5806d083c4273847fd75b202d5d829ea10e05ca94ebfe769d0e94a9b8779da4973be0e640d7b0e0bea44954006a2efe97969d118da3e4
fa7dd88 test: Add test for unknown args (MarcoFalke)

Pull request description:

  Currently uncovered.

  Further reading:

  * https://marcofalke.github.io/btc_cov/total.coverage/src/util/system.cpp.gcov.html
  *  Fail on unknown config file options bitcoin#15021

ACKs for commit fa7dd8:
  promag:
    ACK fa7dd88, tests looks good to me.
  hebasto:
    ACK fa7dd88, I have tested the code.

Tree-SHA512: 86ab370ce8e85925f945a52e81457b5678d71bbabcef01205a97782b780003f363552e0bad1ff678bccc784f82c6b511c3b88de3f8f25f62b0b713c387950564
…nal tests fail

9218ce8 Failing functional tests stop lcov (Aseem Sood)

Pull request description:

  Fixes bitcoin#15648

  Functional tests can fail and lcov still generates a coverage file, which is inaccurate. This change stops `make` from proceeding if functional tests fail.

  before:

  ![image](https://user-images.githubusercontent.com/6106941/59449220-a9a11480-8dd4-11e9-9eff-81c42513aafa.png)

  after:
  ![image](https://user-images.githubusercontent.com/6106941/59449234-b160b900-8dd4-11e9-9d80-6e9c7f41c241.png)

ACKs for commit 9218ce:
  laanwj:
    straightforward enough ACK 9218ce8

Tree-SHA512: 6bbba625f021471d897e911b0df7703153634ef133e295e7be8639346e11f5532bac04e9bab7d793e520fdf4b903219cacecc2ce1e25da0a6828a34a396729e2
eb832cd MSVC: Fix error in debug mode (Fix bitcoin#16245) (nicolas.dorier)

Pull request description:

  This fix a build error in Debug mode bitcoin#16245

ACKs for commit eb832c:
  sipsorcery:
    tACK eb832cd

Tree-SHA512: 6d874afc4ffd8b033ca4eafa40d3bb259bed82b4ca09afa7ed05e3b47c11422503b2f75b91da017bfa9ffc6587fff492cdd76014477ed1d90df326b304d6a77c
The "Git and GitHub tips" section was moved from doc/developer-notes.md to doc/productivity.md in 5b76c31, but the header link to that long-gone section in the developer notes remains and needs to go.

So long, Git and GitHub tips, we barely knew ya.
…ig always returns a legacy address

a495034 Make and get the multisig redeemscript and destination in one function instead of two (Andrew Chow)

Pull request description:

  `CreateMultisigRedeemscript()` is changed to `AddAndGetMultisigDestination()` so that the process of constructing the redeemScript and then getting the `CTxDestination` are done in the same function. This allows that function to see what the keys in the multisig are so that the correct address type is returned from `AddAndGetDestinationForScript()`.

  This only effects the `createmultisig` and `addmultisigaddress` RPCs and does not change signing logic as bitcoin#16022 does.

  Alternative to bitcoin#16022 and bitcoin#16012

  Fixes bitcoin#16011

ACKs for commit a49503:

Tree-SHA512: 5b0154a714deea3b2cc3a54beb420c95eeeacf4ca30c40ca80940d9d640f8b03611b0fc14c2f0710bfd8a79e8d27ad7d9ae380b4b83d52b40ab201624f2a63f0
e61de63 Change ismine to take a CWallet instead of CKeyStore (Andrew Chow)
7c611e2 Move ismine to wallet module (Andrew Chow)

Pull request description:

  `IsMine` isn't used outside of the wallet except for the tests. It also doesn't make sense to be outside of the wallet. This PR moves `IsMine` into the wallet module and for it to take a `CWallet` instead of `CKeyStore`. The test that used `IsMine` is also moved to the wallet tests.

  This is first [prerequisites](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Class-Structure-Changes#ismine) for the wallet structure changes.

ACKs for commit e61de6:
  MarcoFalke:
    re-ACK e61de63 (only change is rebase with git auto-merge)
  meshcollider:
    Very light code review ACK bitcoin@e61de63

Tree-SHA512: 1cb4ad12652aef7922ab7460c6d413e8b9d1855dca78c0a286ae49d5c0765bc7996c55f262c742001d434eb9bd4215dc2cc7aae1b371ee1a82d46b32c17e6341
5a88ea7 doc: remove orphaned header in developer notes (Jon Atack)

Pull request description:

  The "Git and GitHub tips" section was moved from doc/developer-notes.md to doc/productivity.md in 5b76c31, but the header link to that long-gone section in the developer notes remains and needs to go.

  So long, Git and GitHub tips, we barely knew ya.

ACKs for commit 5a88ea:
  fanquake:
    ACK 5a88ea7

Tree-SHA512: d2a0bea27cd9209adec2127fae5e336c44771aa46af7c544fa2c80a3df4868adafbccc30ef370369404b882fa009f7198cbf41777afd30eccf0c21a7eb1d0ad1
qWarning() is used to report warnings and recoverable errors in your 
application.
qInfo() is used for informational messages (since Qt 5.5).
Always show the same help topic regardless of wallet flags, and explain that something is not always available, rather than runtime-modifying the help output.
71d0344 docs: release note wording (Karl-Johan Alm)
3d2ff37 wallet/rpc: use static help text (Karl-Johan Alm)
53c3c1e wallet/rpc/getbalances: add entry for 'mine.used' balance in results (Karl-Johan Alm)

Pull request description:

  This addresses a few remaining issues pointed out in bitcoin#13756:

  * First commit addresses bitcoin#13756 (comment)
  * Second commit addresses bitcoin#13756 (comment)

  Ping jnewbery and achow101 as they pointed out these issues.

ACKs for commit 71d034:
  jnewbery:
    ACK 71d0344
  meshcollider:
    re-utACK bitcoin@71d0344

Tree-SHA512: 5e28822af0574ad07dbbed21aa2fe7866bf5770b4c0a1c150ad0da8af3152bcfb7170330a7497fa500326c594740ecf63733cf58325821e2811d7b911d5783a0
5224be5 gui: Fix open wallet menu initialization order (João Barbosa)

Pull request description:

  Fixes bitcoin#16230, the menu must be created before connecting to aboutToShow signal.

ACKs for commit 5224be:
  hebasto:
    ACK 5224be5, I have tested the code on Bionic with Qt 5.12.4.
  ryanofsky:
    utACK 5224be5. Looks good, fix is simple and makes perfect sense after seeing explanation in bitcoin#16118 (comment). Without this change (and since bitcoin#16118), the menu pointer passed to `connect(m_open_wallet_action->menu(), ...)` is null and connecting has no effect. With this change, the menu is constructed earlier so the connect call can work.
  fanquake:
    ACK 5224be5 Testing included in a comment above. The segfaulting with QT_FATAL_WARNINGS is unrelated to this change.

Tree-SHA512: 97b42493b37b96683058bccf39a0ee93589293d4ba8f0c60aef7f4fb9dd084cc6d5608cd5ef531cadf5e03b1f01627ef96bc2d79f784fb38cb87aa6643183d41
a2aabfb Use qInfo() if no error occurs (Hennadii Stepanov)

Pull request description:

  [Warning and Debugging Messages](https://doc.qt.io/qt-5/debug.html#warning-and-debugging-messages):
  > - `qInfo()` is used for informational messages.
  > - `qWarning()` is used to report warnings and recoverable errors in your
  application.
  >
  > If the `QT_FATAL_WARNINGS` environment variable is set, `qWarning()` exits after printing the warning message. This makes it easy to obtain a backtrace in the debugger.

  [`qWarning()`](https://doc.qt.io/qt-5/qtglobal.html#qWarning):
  > Calls the message handler with the warning message message... This function does nothing if `QT_NO_WARNING_OUTPUT` was defined during compilation; it exits if at the nth warning corresponding to the counter in environment variable `QT_FATAL_WARNINGS`.

  This PR allows more productive debugging using the environment variable `QT_FATAL_WARNINGS`.

  Examples:
  - bitcoin#16118 (comment)
  - bitcoin#16254 (comment)

  The behavior, when option `-debug=qt` is set/unset, remains unchanged.

ACKs for commit a2aabf:
  promag:
    ACK a2aabfb, I also have this change locally.
  Empact:
    ACK bitcoin@a2aabfb
  laanwj:
    ACK a2aabfb
  fanquake:
    ACK a2aabfb.

Tree-SHA512: b4df300c9c00a1705b0d3a10227e3deaac19a98b0a898bb60d5a88872cf450fb131eba150d9dd6c29e021566ee04b3b86b7d486bbe28bd894743c128d2309155
099e4b9 Set AA_EnableHighDpiScaling attribute early (Hennadii Stepanov)

Pull request description:

  Running `bitcoin-qt` compiled against Qt 5.12.4 causes a warning:
  ```
  hebasto@bionic-qt:~/bitcoin$ src/qt/bitcoin-qt
  Attribute Qt::AA_EnableHighDpiScaling must be set before QCoreApplication is created.
  ```

  This PR fixes this issue.

  From Qt docs:
  - [Qt::AA_EnableHighDpiScaling](https://doc.qt.io/qt-5/qt.html#ApplicationAttribute-enum):
  > Enables high-DPI scaling in Qt on supported platforms (see also High DPI Displays). _Supported platforms are X11, Windows and Android._ Enabling makes Qt scale the main (device independent) coordinate system according to display scale factors provided by the operating system. This corresponds to setting the `QT_AUTO_SCREEN​_SCALE_FACTOR` environment variable to 1. This attribute must be set before `QGuiApplication` is constructed. This value was added in Qt 5.6.

  - [QCoreApplication::setAttribute()](https://doc.qt.io/qt-5/qcoreapplication.html#setAttribute)

ACKs for commit 099e4b:
  MarcoFalke:
    ACK 099e4b9
  jonasschnelli:
    utACK 099e4b9
  fanquake:
    ACK 099e4b9. Did some testing on `Bionic` and `Windows 10` (using VirtualBox). I couldn't see any obvious visual difference, but given Marco's screens above, this change is obviously better. I also checked that there wasn't any sort of regression on macOS.

Tree-SHA512: 1965a427ee14ffb3871bac317685032406cf02d1fa2b2dc11c8b643bfe4ba09195674d149d1e41752f14c0d000446b35e142f3ce60d987ba97082fd7ee39a094
Adds member functions to CTxIn and CScript for determining if a
transaction spends any of the SegWit V0 output types.
Count creation of new native P2WSH and P2WPKH outputs, and the number of
transactions creating each type of output.

Count the number of native P2WPKH, native P2WSH, nested P2WSH, and
nested P2WPKH outputs spent. Count the number of transactions that spend
each type of SegWit output.
For each new output, checks whether it would be considered dust at
several different fee-rates.
Adds batching transaction counter.
Tracks the number of outputs created by each transaction.
Adds consolidating transaction counter.
Counts the number of transactions signalling opt-in RBF.
getblockstats now counts the number of transactions with at least 3
inputs and exactly one output as "many-to-one" (mto) consolidations.

It also counts the total number of outputs consolidated by mto
transactions per block.
Tracks total value of many_to_one outputs.
Tracks total value of different kinds of SegWit outputs spent.
Tracks total value of native SegWit outputs created.
@jnewbery
Copy link
Copy Markdown
Author

jnewbery commented Jul 5, 2019

Easier to review here: #3

@jnewbery jnewbery closed this Jul 5, 2019
elichai pushed a commit that referenced this pull request Jan 14, 2020
…ter-return checking

8d22ab0 ci: Enable address sanitizer (ASan) stack-use-after-return checking (practicalswift)

Pull request description:

  Enable address sanitizer (ASan) stack-use-after-return checking (`detect_stack_use_after_return=1`).

  Example:

  ```
  #include <iostream>
  #include <string>

  const std::string& get_string(int i) {
      return std::to_string(i);
  }

  int main() {
      std::cout << get_string(41) << "\n";
  }
  ```

  Without address sanitizer (ASan) stack-use-after-return checking:

  ```
  $ ./stack-use-after-return

  $
  ```

  With address sanitizer (ASan) stack-use-after-return checking:

  ```
  $ ASAN_OPTIONS="detect_stack_use_after_return=1" ./stack-use-after-return
  =================================================================
  ==10400==ERROR: AddressSanitizer: stack-use-after-return on address 0x7f7fa0400030 at pc 0x00000049d2cc bp 0x7ffcbd617070 sp 0x7ffcbd616820
  READ of size 2 at 0x7f7abbecd030 thread T0
      #0 0x439781 in fwrite
      #1 0x7f7ac0504cb3 in std::basic_ostream<char, std::char_traits<char> >& std::__ostream_insert<char, std::char_traits<char> >(std::basic_ostream<char, std::char_traits<char> >&, char const*, long) (/usr/lib/x86_64-linux-gnu/libstdc++.so.6+0x113cb3)
      #2 0x4f9b5f in main stack-use-after-return.cpp:9:15
      #3 0x7f7abf440b96 in __libc_start_main
      #4 0x41bbc9 in _start
  …
  $
  ```

Top commit has no ACKs.

Tree-SHA512: 6557a9ff184023380fd9aa433cdf413e01a928ea99dbc59ec138e5d69cb9e13592e8bb5951612f231ff17a37a895bec5c0940c8db5f328a5c840a5771bdeeba5
elichai pushed a commit that referenced this pull request Jan 14, 2020
1828c6f refactor: Styling w/ clang-format, comment update (Hennadii Stepanov)
88a94f7 qt: Fix missing qRegisterMetaType for size_t (Hennadii Stepanov)

Pull request description:

  On master (a7aec7a) this connection https://github.com/bitcoin/bitcoin/blob/a7aec7ad97949a82f870c033d8fd8b65d772eacb/src/qt/rpcconsole.cpp#L587 fails due to `ClientModel::mempoolSizeChanged()` signal has unregistered parameter type `size_t`: https://github.com/bitcoin/bitcoin/blob/a7aec7ad97949a82f870c033d8fd8b65d772eacb/src/qt/clientmodel.h#L102

  More:
  ```
  $ QT_FATAL_WARNINGS=1 lldb src/qt/bitcoin-qt -- -debug=qt
  ...
  (lldb) bt
  * thread bitcoin#17, name = 'QThread', stop reason = signal SIGABRT
    * frame #0: 0x00007ffff35fce97 libc.so.6`__GI_raise(sig=2) at raise.c:51
      frame #1: 0x00007ffff35fe801 libc.so.6`__GI_abort at abort.c:79
      frame #2: 0x00007ffff5901352 libQt5Core.so.5`QMessageLogger::warning(char const*, ...) const + 354
      frame #3: 0x00007ffff5b216fe libQt5Core.so.5`___lldb_unnamed_symbol2329$$libQt5Core.so.5 + 334
      frame #4: 0x00007ffff5b2456d libQt5Core.so.5`QMetaObject::activate(QObject*, int, int, void**) + 1933
      frame #5: 0x000055555566872e bitcoin-qt`ClientModel::mempoolSizeChanged(this=<unavailable>, _t1=<unavailable>, _t2=<unavailable>) at moc_clientmodel.cpp:260
  ...

  ```

  `debug.log`:
  ```
  [] GUI: QObject::connect: Cannot queue arguments of type 'size_t'
  (Make sure 'size_t' is registered using qRegisterMetaType().)
  ```

  This PR fixes it.

  Refs:
  - [Qt docs: qRegisterMetaType](https://doc.qt.io/qt-5/qmetatype.html#qRegisterMetaType)
  - bitcoin#16348

  ---

  Side NOTE: Also I believe this line https://github.com/bitcoin/bitcoin/blob/a7aec7ad97949a82f870c033d8fd8b65d772eacb/src/qt/bitcoin.cpp#L63 is redundant since long `CAmount` is a `typedef`.

ACKs for top commit:
  laanwj:
    Tested ACK 1828c6f

Tree-SHA512: 2c7f9fe6a5ae70f2e1dd86b07f95d4b00c85c5706a9d722f063f80beb71880d012ec46556963fb1544c2af53d006936c2f7612eae60d9193f67db62ba3d86129
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.