Skip to content
Permalink
Branch: master
Commits on Oct 22, 2019
  1. Merge #17209: tests: Remove no longer needed UBSan suppressions (issu…

    MarcoFalke committed Oct 22, 2019
    …es fixed). Add documentation.
    
    0616138 tests: Remove no longer needed UBSan suppressions (issues fixed). Add documentation. (practicalswift)
    
    Pull request description:
    
      Remove no longer needed UBSan suppressions (issues fixed). Add documentation.
    
      This PR is the CI-only subset of #17208 (which touches code).
    
      From a fuzzing perspective it would be really nice to be able to run UBSan with as few suppressions as possible :)
    
    Top commit has no ACKs.
    
    Tree-SHA512: a926ab3e80e12a805af110fbff470cdc61ef4db536919a5b8896ea8b70f761114a52d9b1c0f48b11c1d48338351bf2e003e01ce60c613612f26ba298dcc29cd9
  2. Merge #17205: ci: Enable address sanitizer (ASan) stack-use-after-ret…

    MarcoFalke committed Oct 22, 2019
    …urn 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
Commits on Oct 21, 2019
  1. tests: Remove no longer needed UBSan suppressions (issues fixed). Add…

    practicalswift committed Oct 21, 2019
    … documentation.
  2. Merge #17070: wallet: Avoid showing GUI popups on RPC errors

    laanwj committed Oct 21, 2019
    facec1c wallet: Avoid showing GUI popups on RPC errors (MarcoFalke)
    
    Pull request description:
    
      RPC errors and warnings are shown as popups in the GUI instead of being returned to the RPC caller. For example,
    
      ```
      $ ./src/bitcoin-cli loadwallet $(pwd)/./test/functional/data/wallets/high_minversion/
      error code: -4
      error message:
      Wallet loading failed.
      ```
    
      gives me a GUI popup and no reason why loading the wallet failed.
    
      After this pull request:
    
      ```
      $ ./src/bitcoin-cli loadwallet $(pwd)/./test/functional/data/wallets/high_minversion/
      error code: -4
      error message:
      Wallet loading failed: Error loading /home/marco/workspace/btc_bitcoin_core/./test/functional/data/wallets/high_minversion/wallet.dat: Wallet requires newer version of Bitcoin Core
    
    ACKs for top commit:
      laanwj:
        Code review ACK facec1c
    
    Tree-SHA512: c8274bbb02cfcf71676eeec1e773e51fb3538cf93f82e7cb8536f4716d44ed819cdc162dfc039ac7386a4db381a734cdb27fd32567043a1180c02519fbcba194
  3. Merge #17195: gui: send amount placeholder value

    laanwj committed Oct 21, 2019
    57e2ede Send amount shows minimum amount placeholder (JeremyCrookshank)
    
    Pull request description:
    
      Noticed that there wasn't a default value for the send amount. However if you put a value in or click the up and down arrows you're unable to get it blank again, so it makes sense that it has a default value. I hope this also makes it more clear that users can send less than 1 BTC if it shows the 8 decimal places
    
      PR:
      ![Capture](https://user-images.githubusercontent.com/46864828/67132088-549c6180-f1ff-11e9-9ba5-67fdcd6db894.PNG)
    
    ACKs for top commit:
      promag:
        ACK 57e2ede.
      GChuf:
        ACK 57e2ede
      laanwj:
        ACK 57e2ede, this is a surprisingly compact solution too
    
    Tree-SHA512: 354590d2a88231b8649f7ae985c8a7864d74ca0e1f8603cb1730ba46747084de90ee6285ce4d39ee04b054fb9cd2d78ebc71146f3af694c37a8a3aff7f051800
  4. Merge #17176: ci: Cleanup macOS runs

    laanwj committed Oct 21, 2019
    fa677d1 ci: Remove redundant check for TRAVIS_OS_NAME (MarcoFalke)
    fadccb2 doc: Document that GNU tools are required for linters (MarcoFalke)
    4444704 ci: Cleanup macOS runs (MarcoFalke)
    
    Pull request description:
    
      * Remove a commented out cleanup task in `before_cache`
      * Remove the linter run on macOS, and document that GNU tools are required to run the linters
    
    ACKs for top commit:
      Sjors:
        Code review ACK fa677d1
      laanwj:
        ACK fa677d1
      ryanofsky:
        Code review ACK fa677d1 for new third commit replacing TRAVIS_OS_NAME check with NO_DEPENDS setting
    
    Tree-SHA512: 9122a63bbe7887d9e379123152ea4ba44324cb18033b9e6b45bfdb1af665c10ea598564b9fcd57330d208a08e4696e41b4d6175f05f0843a3a76530da114f8c6
  5. Merge #17191: random: remove call to RAND_screen() (Windows only)

    laanwj committed Oct 21, 2019
    e892f96 random: remove call to RAND_screen() (Windows only) (fanquake)
    
    Pull request description:
    
      Follow up to #17151 where there were multiple calls to also remove our call to RAND_screen().
    
    ACKs for top commit:
      MarcoFalke:
        unsigned ACK e892f96
      laanwj:
        ACK e892f96
    
    Tree-SHA512: 1b846016d91e8113f90466b61fcaf0574edb6b4726eba1947549e2ac28907e1318d893f7b303e756f19730c8507c79b10e08d54b97153224b585ff1e0ac1953e
Commits on Oct 20, 2019
  1. Merge #17203: wallet: Remove unused GetLabelName

    fanquake committed Oct 20, 2019
    7ca68e1 wallet: Remove unused GetLabelName (Sebastian Falbesoner)
    
    Pull request description:
    
      While taking a look at #17198 I noticed that the method `CWallet::GetLabelName(...)` is not used anymore, since the `account` API was removed (c9c32e6).
    
    ACKs for top commit:
      practicalswift:
        ACK 7ca68e1
      promag:
        ACK 7ca68e1.
      fanquake:
        ACK 7ca68e1
    
    Tree-SHA512: 6825d77a85934e2368a3fb44c8db0ed0872aa5606e3761decb0a6b7e3773277afa7021bf1f71009207c3961cdd0a1c448854ea2fd8be95c3afec466254faf82d
  2. wallet: Remove unused GetLabelName

    theStack committed Oct 20, 2019
  3. Merge #17157: doc: Added instructions for how to add an upsteam to fo…

    fanquake committed Oct 20, 2019
    …rked repo
    
    f09ba06 doc: Added instructions for how to add an upsteam to forked repo (dannmat)
    
    Pull request description:
    
      As a first time git developer, I struggled to understand whether to create a new fork for each pull request or not.
    
      After asking the IRC chat, I have added this to the documentation to further help new developers using git.
    
    ACKs for top commit:
      fanquake:
        ACK f09ba06 - For such a simple change, I think we've bike-shed this enough already. The `bitcoin/bitcoin` repo isn't really where anyone should be learning how to use `git` etc, but I think linking out here is ok.
    
    Tree-SHA512: e0e9d655d0725e0128673afedb81dc5ba9387968fcbb681de7e50155a2cfa1a7f39fad040b596f4de9ad6727a1a8a90fd3d36eaa5242bc12186c3b82abd23fb2
  4. doc: Added instructions for how to add an upsteam to forked repo

    dannmat and fanquake committed Oct 15, 2019
    As a first time git developer, I struggled to understand whether to create a new fork for each pull request or not.
    
    After asking the IRC chat, I have added this to the documentation to further help new developers using git.
    
    Co-Authored-By: Michael <fanquake@gmail.com>
Commits on Oct 19, 2019
  1. Send amount shows minimum amount placeholder

    JeremyCrookshank committed Oct 19, 2019
Commits on Oct 18, 2019
  1. Merge #17184: util: Filter out macOS process serial number

    fanquake committed Oct 18, 2019
    b5f0be3 util: Filter out macOS process serial number (Hennadii Stepanov)
    
    Pull request description:
    
      Fix #17179
    
    ACKs for top commit:
      laanwj:
        ACK b5f0be3
      MarcoFalke:
        unsigned ACK b5f0be3
      promag:
        ACK b5f0be3.
      fanquake:
        ACK b5f0be3 - Tested that this fixes #17179.
    
    Tree-SHA512: 84ce859e53ebc7ad2d0a45e954243ef6efee640f1e0212322f68a317e4361a216ecb4b5a3a410ab31613adc285c8d3840fbf41fa9da9019be3d734db6b9427cd
  2. Merge #16949: build: only pass --disable-dependency-tracking to packa…

    fanquake committed Oct 18, 2019
    …ges that understand it
    
    1ba49bc build: pass --enable-option-checking to applicable packages (fanquake)
    bcff8e2 build: only pass --disable-dependency-tracking to packages that understand it (fanquake)
    
    Pull request description:
    
      By blanket passing `--disable-dependency-tracking` to all depends packages we end up with warnings (i.e in `bdb` or `freetype`) like:
      ```bash
      configure: WARNING: unrecognized options: --disable-dependency-tracking
      ```
      Instead, only pass it to packages that actually understand it. Related to #16354.
    
      More info on `--disable-dependency-tracking` available [here](https://www.gnu.org/software/automake/manual/html_node/Dependency-Tracking.html).
    
      This PR also adds `--enable-option-checking` as a configure option to all applicable packages.
    
    ACKs for top commit:
      laanwj:
        ACK 1ba49bc
      theuni:
        ACK 1ba49bc
    
    Tree-SHA512: 6d3143ad5f5d1abed5e0a0b2ffbb4323f21c7bf24b0b8df26fb1b3cd16cf5309bbb830aa5aaec99164d5bbe8e9c62b97aa3e97ee1ddc2c7612bf8ff88a63885e
  3. random: remove call to RAND_screen() (Windows only)

    fanquake committed Oct 18, 2019
    Follow up to #17151 where
    there were multiple calls to also remove our call to RAND_screen().
  4. Merge #17151: gui: remove OpenSSL PRNG seeding (Windows, Qt only)

    fanquake committed Oct 18, 2019
    cc3b528 gui: remove OpenSSL PRNG seeding (Windows, Qt only) (fanquake)
    
    Pull request description:
    
      This removes the code introduced in [#4399](#4399) that attempts to add additional entroy to the OpenSSL PRNG using `RAND_event()`. This is specific to bitcoin-qt running on Windows.
    
      ```
      RAND_event() collects the entropy from Windows events such as mouse movements and other user interaction.
      It should be called with the iMsg, wParam and lParam arguments of all messages sent to the window procedure.
      It will estimate the entropy contained in the event message (if any), and add it to the PRNG.
      The program can then process the messages as usual.
      ```
    
      Besides BIP70, this is the last place we are directly using OpenSSL in the GUI code. All other OpenSSL usage is in [random.cpp](https://github.com/bitcoin/bitcoin/blob/master/src/random.cpp).
    
      Note that we are still also still doing other Windows specific gathering using [RandAddSeedPerfmon](https://github.com/bitcoin/bitcoin/blob/master/src/random.cpp#L268) and [RAND_screen()](https://github.com/bitcoin/bitcoin/blob/master/src/random.cpp#L600) on top of the other generation we do.
    
      Also note that if RAND_event returns `0` here (PRNG has **NOT** been seeded with enough data), we're just logging a single message and continuing, which also seems less than ideal.
    
    ACKs for top commit:
      laanwj:
        ACK cc3b528
      MarcoFalke:
        unsigned ACK cc3b528
      theuni:
        ACK cc3b528.
    
    Tree-SHA512: 0bb18779cf37f6670e3e5ac6a6a38e5f95199491b2684f9e56391c76f030fe1621d6df064239c2a398f228129fdf3f2220fc8cd15b2b92ecf2ea6d98a79b2175
  5. Merge #16889: Add some general std::vector utility functions

    MarcoFalke committed Oct 18, 2019
    7d8d3e6 Add tests for util/vector.h's Cat and Vector (Pieter Wuille)
    e65e61c Add some general std::vector utility functions (Pieter Wuille)
    
    Pull request description:
    
      This is another general improvement extracted from #16800 .
    
      Two functions are added are:
    
      * Vector(arg1,arg2,arg3,...) constructs a vector with the specified arguments as elements. The vector's type is derived from the arguments. If some of the arguments are rvalue references, they will be moved into place rather than copied (which can't be achieved using list initialization).
      * Cat(vector1,vector2) returns a concatenation of the two vectors, efficiently moving elements when relevant.
    
      Vector generalizes (and replaces) the `Singleton` function in src/descriptor.cpp, and `Cat` replaces the function in bech32.cpp
    
    ACKs for top commit:
      laanwj:
        ACK 7d8d3e6
      MarcoFalke:
        ACK 7d8d3e6 (enjoyed reading the tests, but did not compile)
    
    Tree-SHA512: 92325f14e90d7e7d9d920421979aec22bb0d730e0291362b4326cccc76f9c2d865bec33a797c5c0201773468c3773cb50ce52c8eee4c1ec1a4d10db5cf2b9d2a
  6. Merge #17186: gui: Add placeholder text to the sign message field

    fanquake committed Oct 18, 2019
    7005d6a gui: Add placeholder text to the sign message field (Danny-Scott)
    
    Pull request description:
    
      When using the sign message functionality I noticed the "message" field had no label or placeholder text to highlight what it's for.
    
      I've added the placeholder text to match the tool tip to help it be more user friendly.
    
    ACKs for top commit:
      hebasto:
        Re-ACK 7005d6a
      fanquake:
        ACK 7005d6a
    
    Tree-SHA512: 17fe51c134f6373d8d5f9ca98b15bd936da4e61aa5258ceb5d318575d49b43cbfde6f4c3f720eb5928206902e6ba52811ba08737a03c95224e45dabc947d9d11
  7. gui: Add placeholder text to the sign message field

    Danny-Scott authored and root committed Oct 16, 2019
  8. Merge #15084: gui: don't disable the sync overlay when wallet is disa…

    jonasschnelli committed Oct 18, 2019
    …bled
    
    b3b6b6f gui: don't disable the sync overlay when wallet is disabled (Ben Carman)
    
    Pull request description:
    
      Continuation of #13848.
    
      When running with `-disablewallet` the sync modal is now available by clicking on the progress bar or `syncing` icon.
    
      [Current Image of what the window looks like](https://imgur.com/6LsoT2l)
    
      Fixes #13828.
    
    ACKs for top commit:
      jonasschnelli:
        Tested ACK b3b6b6f
    
    Tree-SHA512: 325bc22a0b692bfb8fcc9d84e02dfc506146028b97b3609e23c2c45288c79b8aead1ad2e9b8d692f5f6771b4d2aee63fbe71bfaeaf17d260865da32ab3631e07
  9. util: Filter out macOS process serial number

    hebasto committed Oct 18, 2019
Commits on Oct 17, 2019
  1. Merge #17162: chain: Remove CBlockIndex::SetNull helper

    fanquake committed Oct 17, 2019
    fa04673 chain: Set all CBlockIndex members to null, remove SetNull helper (MarcoFalke)
    
    Pull request description:
    
      The first commit removes the `SetNull` helper and inlines the member initialization (C++11). See https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#c-data-structures for rationale.
    
      <strike>The second commit adds the `cs_main` lock annotation to `RaiseValidity`. See also #17161.</strike>
    
    ACKs for top commit:
      promag:
        Code review ACK fa04673.
      practicalswift:
        ACK fa04673 -- diff still looks correct :)
      laanwj:
        ACK fa04673, this makes it easy to see that all fields are initialized.
    
    Tree-SHA512: 1b2b9fb0951c03c75b9cce322b89d4ecc9a364ae78b94d91b0b4669437824394dfada820ab6f74dfac3193f602899abfdc244ae2d9351ad293f555488f03470e
  2. Merge #17177: doc: Describe log files + consistent paths in test READMEs

    MarcoFalke committed Oct 17, 2019
    9576614 doc: Describe log files + consistent paths in test READMEs (Martin Erlandsson)
    
    Pull request description:
    
      picks up #15830
    
      I saw this was almost ready to merge but the test logging part was not 100% correct. I reworked that part, the rest is the same.
    
    ACKs for top commit:
      GChuf:
        ACK 9576614
    
    Tree-SHA512: 3de7f1b0a1b0419df6e7b55964d00e715b6cb7874b1849ad6f120597610d7df4182c4b61b9c9691ce04f4e392ed3caead4c623374be2066ac31319e702d45d09
  3. doc: Describe log files + consistent paths in test READMEs

    merland authored and fjahr committed Apr 16, 2019
  4. ci: Remove redundant check for TRAVIS_OS_NAME

    MarcoFalke committed Oct 17, 2019
    Can be reviewed with
    git diff --ignore-all-space --function-context
  5. doc: Document that GNU tools are required for linters

    MarcoFalke committed Oct 17, 2019
  6. ci: Cleanup macOS runs

    MarcoFalke committed Oct 17, 2019
  7. Merge #16597: Travis: run full test suite on native macOS

    MarcoFalke committed Oct 17, 2019
    1f6c650 travis: run tests on macOS native (Sjors Provoost)
    
    Pull request description:
    
      Adds an additional Travis machine to run the functional test suite on native macOS
    
      Homebrew is not particularly Travis compatible, but I found some useful hints here: https://discourse.brew.sh/t/best-practice-for-homebrew-on-travis-brew-update-is-5min-to-build-time/5215/11
    
    ACKs for top commit:
      MarcoFalke:
        re-ACK 1f6c650
    
    Tree-SHA512: 3f19a1695fac53d4d6c2033a9c20be69294e3a798c84fd9bf6ae2aa7a6d92aa1dad1f62f4ee1ada9413fe7d05ee974050fa030fd2c547f33e0d5c0a3e74f64db
  8. Merge #17119: doc: Fix broken bitcoin-cli examples

    laanwj committed Oct 17, 2019
    85016e5 [rpc] Fix broken bitcoin-cli examples (Andrew Toth)
    
    Pull request description:
    
      This fixes the `bitcoin-cli` examples for `combinerawtransaction`, `combinepsbt` and `testmempoolaccept`. They currently return `Error parsing JSON`.
    
    ACKs for top commit:
      laanwj:
        ACK 85016e5
    
    Tree-SHA512: b561f68f7a188dc91dab1ceb98da3ac3e232143ab2b906c90f95c6b74b584599d0f3b51f067cdd3b1153931f95b3dc385e453b1a0dde86f9cb549b94560f219d
  9. Merge #17169: doc: correct function name in ReportHardwareRand()

    MarcoFalke committed Oct 17, 2019
    5013171 doc: correct function name in ReportHardwareRand() (fanquake)
    
    Pull request description:
    
      The function is `InitHardwareRand` not `HWRandInit`.
    
      https://github.com/bitcoin/bitcoin/blob/46d6930f8c7ba7cbcd7d86dd5d0117642fcbc819/src/random.cpp#L99
    
    ACKs for top commit:
      laanwj:
        ACK 5013171
      theStack:
        ACK 5013171
    
    Tree-SHA512: c25e1bb56e923961fc8a9178d751222b60f5ca36be84abf8fd1ac971f3a9b79b587ed9d8a4a175981b66f3fd5ad7edd6697d343e4dc4852351a1510718745455
  10. Merge #17140: test: Fix bug in blockfilter_index_tests.

    MarcoFalke committed Oct 17, 2019
    f59bbb6 test: Fix bug in blockfilter_index_tests. (Jim Posen)
    
    Pull request description:
    
      The test case tests a chain reorganization, however the two chains were generated in the same manner and thus produced the same blocks.
    
      This issue was [pointed out](#14121 (comment)) by MarcoFalke.
    
    ACKs for top commit:
      MarcoFalke:
        Thanks! ACK f59bbb6 (looked at the diff on GitHub, didn't compile, nor run tests)
    
    Tree-SHA512: a2f063ae9312051ffc2a3fcc1116a6a8ac09beeef261bc40aa3ff7270ff4de22a790eb19fec6b15ba1eb46e78f1f317bfd91472d8581b95bb9441a56b102554e
  11. doc: correct function name in ReportHardwareRand()

    fanquake committed Oct 17, 2019
Commits on Oct 16, 2019
  1. Merge #16659: refactoring: Remove unused includes

    MarcoFalke committed Oct 16, 2019
    084e17c Remove unused includes (practicalswift)
    
    Pull request description:
    
      As requested by MarcoFalke in #16273 (comment):
    
      This PR removes unused includes.
    
      Please note that in contrast to #16273 I'm limiting the scope to the trivial cases of pure removals (i.e. no includes added) to make reviewing easier.
    
      I'm seeking "Concept ACK":s for this obviously non-urgent minor cleanup.
    
      Rationale:
      * Avoids unnecessary re-compiles in case of header changes.
      * Makes reasoning about code dependencies easier.
      * Reduces compile-time memory usage.
      * Reduces compilation time.
      * Warm fuzzy feeling of being lean :-)
    
    ACKs for top commit:
      ryanofsky:
        Code review ACK 084e17c. PR only removes include lines and it still compiles. In the worst case someone might have to explicitly add an include later for something now included implicitly. But maybe some effort was taken to avoid this, and it wouldn't be a tragedy anyway.
    
    Tree-SHA512: 89de56edc6ceea4696e9579bccff10c80080821685b9fb4e8c5ef593b6e43cf662f358788701bb09f84867693f66b2e4db035b92b522a0a775f50b7ecffd6a6d
Older
You can’t perform that action at this time.