Commit Graph

17708 Commits

Author SHA1 Message Date
MarcoFalke
cbe439629e Merge bitcoin-core/gui#167: raise helpMessageDialog
77114462f2 raise helpMessageDialog (randymcmillan)

Pull request description:

  the raise() method brings the helpMessageDialog to the top if it is obscured by another window.

ACKs for top commit:
  promag:
    Code review ACK 77114462f2.
  hebasto:
    ACK 77114462f2, tested on:

Tree-SHA512: 0d5b107aa9a5ce3891e88ef69f64461c8b23d17476b798691119e84bfc78e16b2491c798adb5d6cc347af3b7f18729593d7924090c336114a3cf34fbee344bfb
2021-01-26 09:47:52 +01:00
fanquake
f827e151a2 refactor: remove straggling boost::mutex usage
After the merge of #18710, the linter is warning:
```bash
A new Boost dependency in the form of "boost/thread/mutex.hpp" appears to have been introduced:
src/sync.cpp:#include <boost/thread/mutex.hpp>
src/test/sync_tests.cpp:#include <boost/thread/mutex.hpp>

^---- failure generated from test/lint/lint-includes.sh
```

the interim #19337 was merged, which introduced more `boost::mutex` usage.

Given we no longer use `boost::mutex`, just remove the double lock test
and remaining includes.
2021-01-26 15:57:28 +08:00
Samuel Dobson
16ae3368f2 Merge #17350: doc: Add developer documentation to isminetype
40f05647ee doc: Add developer documentation to isminetype (HAOYUatHZ)

Pull request description:

  Closes: https://github.com/bitcoin/bitcoin/issues/17217

ACKs for top commit:
  meshcollider:
    utACK 40f05647ee

Tree-SHA512: 156ff3bc02613d65aed5fcf50250ec3f3365b6c83c810763673ecfdd081a1310e5235be05f0c782638f191be61ad0028511392c40e4106a56eb1c6a3a8ab73b9
2021-01-26 13:24:35 +13:00
Samuel Dobson
4b15ffe991 Merge #20832: rpc: Better error messages for invalid addresses
8f0b64fb51 Better error messages for invalid addresses (Bezdrighin)

Pull request description:

  This PR addresses #20809.

  We add more detailed error messages in case an invalid address is provided inside the 'validateaddress' and 'getaddressinfo' RPC calls. This also covers the case when a user provides an address from a wrong network.

  We also add a functional test to test the new error messages.

ACKs for top commit:
  kristapsk:
    ACK 8f0b64fb51
  meshcollider:
    Code review ACK 8f0b64fb51

Tree-SHA512: ca0f806ab573e96b79e98d9f8c810b81fa99c638d9b5e4d99dc18c8bd2568e6a802ec305fdfb2983574a97a19a46fd53b77645f8078fb77e9deb24ad2a22cf93
2021-01-26 13:15:13 +13:00
Wladimir J. van der Laan
b386d37360 Merge #18710: Add local thread pool to CCheckQueue
bb6fcc75d1 refactor: Drop boost::thread stuff in CCheckQueue (Hennadii Stepanov)
6784ac471b bench: Use CCheckQueue local thread pool (Hennadii Stepanov)
dba30695fc test: Use CCheckQueue local thread pool (Hennadii Stepanov)
01511776ac Add local thread pool to CCheckQueue (Hennadii Stepanov)
0ef938685b refactor: Use member initializers in CCheckQueue (Hennadii Stepanov)

Pull request description:

  This PR:
  - gets rid of `boost::thread_group` in the `CCheckQueue` class
  - allows thread safety annotation usage in the `CCheckQueue` class
  - is alternative to #14464 (https://github.com/bitcoin/bitcoin/pull/18710#issuecomment-616618525, https://github.com/bitcoin/bitcoin/pull/18710#issuecomment-617291612)

  Also, with this PR (I hope) it could be easier to resurrect a bunch of brilliant ideas from #9938.

  Related: #17307

ACKs for top commit:
  laanwj:
    Code review ACK bb6fcc75d1
  LarryRuane:
    ACK bb6fcc75d1
  jonatack:
    Code review ACK bb6fcc75d1 and verified rebase to master builds cleanly with unit/functional tests green

Tree-SHA512: fddeb720d5a391b48bb4c6fa58ed34ccc3f57862fdb8e641745c021841c8340e35c5126338271446cbd98f40bd5484f27926aa6c3e76fa478ba1efafe72e73c1
2021-01-25 20:21:19 +01:00
randymcmillan
77114462f2 raise helpMessageDialog 2021-01-25 10:57:44 -05:00
Martin Ankerl
7487bc9900 Fix BlockToJsonVerbose benchmark
Currently it was not possible to run just the BlockToJsonVerboes benchmarsk because it did not set up everything it needed, running `bench_bitcoin -filter=BlockToJsonVerbose` caused this assert to fail:

```
bench_bitcoin: chainparams.cpp:506: const CChainParams& Params(): Assertion `globalChainParams' failed.
```

Initializing TestingSetup fixes this.
2021-01-24 10:31:13 +01:00
Bezdrighin
8f0b64fb51 Better error messages for invalid addresses
This commit addresses #20809.

We add an additional 'error' property in the result of 'validateaddress' in case the address is not valid that gives a short description of why the address in invalid. We also change the error message returned by 'getaddressinfo' in case the address is invalid.
2021-01-24 02:44:53 +01:00
MarcoFalke
32b191fb66 Merge #20927: [refactor] [net] Clean up InactivityCheck()
bf100f8170 [net] Cleanup InactivityChecks() and add commenting about time (John Newbery)
06fa85cd50 [net] InactivityCheck() takes a CNode reference (John Newbery)

Pull request description:

  This is a pure refactor and should not change any behavior. It clarifies and documents the InactivityCheck() function

  This makes #20721 easier to review. In particular, this function uses a mixture of (unmockable) system time and mockable time. It's important to understand where those are being used when reviewing #20721.

  #20721 doesn't require this change, so if others don't agree that it's useful and makes review easier, then I'm happy to close this and just do #20721 directly.

ACKs for top commit:
  fanquake:
    ACK bf100f8170
  MarcoFalke:
    review ACK bf100f8170 💫

Tree-SHA512: 7b001de2a5fbe8a6dc37baeae930db5775290afb2e8a6aecdf13161f1e5b06ef813bc6291d8ee5cefcf1e430c955ea702833a8db84192eebe6e6acf0b9304cb2
2021-01-22 13:13:00 +01:00
fanquake
4bd586607d Merge #20523: zmq: deduplicate 'sequence' publisher message creation/sending
962444295d zmq: deduplicate 'sequence' publisher message creation/sending (Sebastian Falbesoner)

Pull request description:

  This small PR deduplicates common low-level creation and sending code for the 'sequence' zmq publisher message (methods `NotifyBlock{Connect,Disconnect}()`, `NotifyTransaction{Acceptance,Removal}()` in the class `CZMQPublishSequenceNotifier`) by introducing a helper function.

ACKs for top commit:
  jonatack:
    Code review re-ACK 962444295d per `git diff f231ffd 9624442`
  instagibbs:
    utACK 962444295d

Tree-SHA512: de0750d923f36d1a5751331e88eec8a1605cb88c93318830913210485e2bff712310484f18a0fb626df6ef32ce0b0cf57f4421ce62656e05fce7011a0c3c2d0e
2021-01-22 12:58:47 +08:00
fanquake
019aa248d9 Merge #17920: guix: Build support for macOS
f1694757dd guix: Fix typo (Carl Dong)
771c4b98a8 guix: README: Add darwin HOSTS entry (Carl Dong)
8dbf18cb1d guix: Check for macOS SDK before building anything (Carl Dong)
34b23f597e guix: Set ZERO_AR_DATE for darwin build determinism (Carl Dong)
f3835dc6a3 build: Make xorrisofs reproducible with -volume_date (Carl Dong)
c9eb4cf3a0 guix: Add support for darwin builds (Carl Dong)
37fe73a092 build: Add var printing target to src/Makefile.am (Carl Dong)

Pull request description:

  This PR brings our Guix builds on par with Gitian in terms of supported architectures.

  Reviewers: if you run a build, please submit:

  ```
  find output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
  ```

  So that we can compare hashes and ensure reproducibility!

ACKs for top commit:
  fanquake:
    ACK f1694757dd - I think we can make some small usability improvements, but this is ok to merge now.

Tree-SHA512: 4af2b71654a9736467dcc681d10601c6eee37800d7847011a50585455b67b55d61742ca5604585f310a2fd75335b674e5e27dfb5169cb2f26e112aa4c411d8be
2021-01-22 12:16:57 +08:00
MarcoFalke
7f653c3b22 Merge bitcoin-core/gui#176: Fix TxViewDelegate layout
af58f5b12c qt: Stop the effect of hidden widgets on the size of QStackedWidget (Hennadii Stepanov)
f0d04795e2 qt: Fix TxViewDelegate layout (Hennadii Stepanov)
d439921406 qt: Add TransactionOverviewWidget class (Hennadii Stepanov)

Pull request description:

  This change:
  - prevents overlapping date and amount strings
  - guaranties that "eye" sign at the end of the watch-only address/label is always visible

  Fix https://github.com/bitcoin/bitcoin/issues/20826

  Here are some screenshots with this PR with the _minimum available width_ of the transaction list widget:

  ![Screenshot from 2021-01-03 20-23-56](https://user-images.githubusercontent.com/32963518/103486411-6408ca00-4e06-11eb-9c21-627a65e532c1.png)
  ![Screenshot from 2021-01-03 20-24-47](https://user-images.githubusercontent.com/32963518/103486413-6834e780-4e06-11eb-8221-478d98bbdf69.png)
  ![Screenshot from 2021-01-03 20-25-27](https://user-images.githubusercontent.com/32963518/103486418-6d923200-4e06-11eb-8625-a4ed3089b6ab.png)
  ![Screenshot from 2021-01-03 20-33-20](https://user-images.githubusercontent.com/32963518/103486420-708d2280-4e06-11eb-90c2-f2463fb3c4b3.png)

ACKs for top commit:
  dooglus:
    ACK af58f5b.
  jarolrod:
    re-ACK af58f5b12c

Tree-SHA512: 6dae682490ec50fa0335d220bc2d153fa3e6ed578f07c6353a3b180f8f6cf1c2f9e52ebd7b3076f51d7004d86bf5cca14e6b5db9cdf786e85a57a81eacbb4988
2021-01-21 18:54:19 +01:00
MarcoFalke
53bbbe5a20 Merge bitcoin-core/gui#171: Use layout manager for Create Wallet dialog
d4feb6812a qt: Use layout manager for Create Wallet dialog (Hennadii Stepanov)

Pull request description:

  On master (e75f91eae3) not using layout manager causes problems with resizing:

  ![Screenshot from 2021-01-01 13-03-13](https://user-images.githubusercontent.com/32963518/103437728-ce1d4580-4c33-11eb-8915-1e9482775653.png)
  ![Screenshot from 2021-01-01 13-03-26](https://user-images.githubusercontent.com/32963518/103437730-d6758080-4c33-11eb-9e0f-87d0dd487fcb.png)

  Also text labels are not resized properly on some window managers (https://github.com/bitcoin/bitcoin/issues/20777), or if their lengths are changed (after translation).

  This PR introduces a standard layout manager for the "Create Wallet" dialog that fixes all layout issues (actually, the `createwalletdialog.ui` has been re-written from scratch):

  ![Screenshot from 2021-01-01 13-10-03](https://user-images.githubusercontent.com/32963518/103437822-d0cc6a80-4c34-11eb-84fd-fcb10a16d9ef.png)
  ![Screenshot from 2021-01-06 23-50-36](https://user-images.githubusercontent.com/32963518/103823090-0b416780-507a-11eb-89dd-3f48a358e168.png)

  Additional visual changes:
  - advanced options are grouped in `QGroupBox` (https://github.com/bitcoin-core/gui/pull/96#issuecomment-726337165)
  - enabled the [size grip](https://doc.qt.io/qt-5/qsizegrip.html#details)

  Fix https://github.com/bitcoin/bitcoin/issues/20777

ACKs for top commit:
  jarolrod:
    ACK d4feb6812a
  Sjors:
    re-tACK d4feb6812a
  promag:
    Tested ACK d4feb6812a on macos.

Tree-SHA512: 4c055962e49f88624900b880b33a866976d224628784593428b712d2e94563d77ddefddea3397134d20e72f738a8cf9aa885c1272fd9ffc90213c104435fb9f4
2021-01-21 18:51:17 +01:00
MarcoFalke
45952dab9d Merge #20932: refactor: Replace fs::absolute calls with AbsPathJoin calls
da9caa1ced Replace fs::absolute calls with AbsPathJoin calls (Kiminuo)
66576c4fd5 test: Clear forced -walletdir setting after wallet init_tests (Kiminuo)

Pull request description:

  This adds better test coverage and will make it easier in #20744 to remove our dependency on the two-argument boost::filesystem::absolute() function which does not have a direct equivalent in C++17.

  This PR doesn't change behavior aside from adding an assert and fixing a test bug.

ACKs for top commit:
  jonatack:
    Code review ACK da9caa1ced only doxygen improvements since my last review per `git diff d867d7a da9caa1`
  MarcoFalke:
    review ACK da9caa1ced 📯
  ryanofsky:
    Code review ACK da9caa1ced. Just comment and test tweaks since previous review.

Tree-SHA512: c940ee60f3ba374d4927cf34cf12d27c4c735c94af591fbc0ca408c641b30f8f8fbcfe521d66bfbddf9877a1fc8cd99bd8a47ebcd2fa59789de6bd87a7b9cf4d
2021-01-21 18:48:03 +01:00
Carl Dong
37fe73a092 build: Add var printing target to src/Makefile.am
See 181989f6c9 for more info. I missed
this one last time.
2021-01-21 10:57:31 -05:00
MarcoFalke
1f45e85509 Merge #20972: locks: Annotate CTxMemPool::check to require cs_main
b396467053 locks: Annotate CTxMemPool::check to require cs_main (Carl Dong)

Pull request description:

  ```
  Currently, CTxMemPool::check locks CTxMemPool's own cs member, then
  calls GetSpendHeight which locks cs_main. This can potentially cause an
  undesirable lock invesion since CTxMemPool's cs is supposed to be locked
  after cs_main.

  This does not cause us any problems right now because all callers of
  CTxMemPool already lock cs_main before calling CTxMemPool::check, which
  means that the LOCK(cs_main) in GetSpendHeight becomes benign.

  However, it is currently possible for new code to be added which calls
  CTxMemPool::check without locking cs_main (which would be dangerous).
  Therefore we should make it explicit that cs_main needs to be held
  before calling CTxMemPool::check.

  NOTE: After all review-only assertions are removed in "#20158 |
        tree-wide: De-globalize ChainstateManager", and assuming that we
        keep the changes in "validation: Pass in spendheight to
        CTxMemPool::check", we can re-evaluate to see if this annotation
        is still necessary.
  ```
  -----

  Previous discussions:
  1. https://github.com/bitcoin/bitcoin/pull/20158#discussion_r520639845
  2. https://github.com/bitcoin/bitcoin/pull/20158#pullrequestreview-557117202
  3. https://github.com/bitcoin/bitcoin/pull/20749#discussion_r559425521

ACKs for top commit:
  jnewbery:
    Code review ACK b396467053
  MarcoFalke:
    ACK b396467053
  jonatack:
    ACK b396467053 review and debug built, verified that `cs_main` is held by callers of `CTxMemPool::check()` in `PeerManagerImpl::ProcessOrphanTx()`, `PeerManagerImpl::ProcessMessage()`, and `CChainState::ActivateBestChainStep()`

Tree-SHA512: 4635cddb4aa1af9532bb657b2f9c4deec4568d16ba28c574eae91bb77368cd40e23c3c720a9de11cec78e7ad678a44a5e25af67f13214b86b56e777e0c35a026
2021-01-21 16:45:45 +01:00
MarcoFalke
85fee49c39 Merge #20946: fuzz: Consolidate fuzzing TestingSetup initialization
abb6fa7285 fuzz: Initialize a full TestingSetup where appropriate (Carl Dong)
713314abfa fuzz: Consolidate fuzzing TestingSetup initialization (Carl Dong)

Pull request description:

  ```
  Previously, the {Basic,}TestingSetup for fuzzers were set up in many ways:

  1. Calling InitializeFuzzingContext, which implicitly constructs a static
     const BasicTestingSetup
  2. Directly constructing a static const BasicTestingSetup in the initialize_*
     function
  3. Directly constructing a static TestingSetup and reproducing the
     initialization arguments (I'm assuming because
     InitializeFuzzingContext only initializes a BasicTestingSetup)

  The new, relatively-simple MakeFuzzingContext function allows us to
  consolidate these methods of initialization by being flexible enough to
  be used in all situations. It:

  1. Is templated so that we can choose to initialize any of
     the *TestingSetup classes
  2. Has sane defaults which are often used in fuzzers but are also
     easily overridable
  3. Returns a unique_ptr, explicitly transferring ownership to the caller
     to deal with according to its situation
  ```

  ~~Question for fuzzing people: was it intentional that `src/test/fuzz/net.cpp` would directly instantiate the `BasicTestingSetup` and thus omit the `"-nodebuglogfile"` flag?~~ [Answered](https://github.com/bitcoin/bitcoin/pull/20946#issuecomment-761537108)

ACKs for top commit:
  MarcoFalke:
    ACK abb6fa7285

Tree-SHA512: 96a5ca6f4cd5ea0e9483b60165b31ae3e9003918c700a7f6ade48010f419f2a6312e10b816b3187f1d263798827571866e4c4ac0bbfb2e0c79dfad254cda68e7
2021-01-21 16:04:31 +01:00
Carl Dong
abb6fa7285 fuzz: Initialize a full TestingSetup where appropriate
A full TestingSetup is required for both coins_view and
load_external_block_file as they interact with the active chainstate.
2021-01-21 09:29:42 -05:00
Carl Dong
713314abfa fuzz: Consolidate fuzzing TestingSetup initialization
Previously, the {Basic,}TestingSetup for fuzzers were set up in many ways:

1. Calling InitializeFuzzingContext, which implicitly constructs a static
   const BasicTestingSetup
2. Directly constructing a static const BasicTestingSetup in the initialize_*
   function
3. Directly constructing a static TestingSetup and reproducing the
   initialization arguments (I'm assuming because
   InitializeFuzzingContext only initializes a BasicTestingSetup)

The new, relatively-simple MakeFuzzingContext function allows us to
consolidate these methods of initialization by being flexible enough to
be used in all situations. It:

1. Is templated so that we can choose to initialize any of
   the *TestingSetup classes
2. Has sane defaults which are often used in fuzzers but are also
   easily overridable
3. Returns a unique_ptr, explicitly transferring ownership to the caller
   to deal with according to its situation
2021-01-21 09:29:42 -05:00
Sebastian Falbesoner
962444295d zmq: deduplicate 'sequence' publisher message creation/sending 2021-01-21 14:35:21 +01:00
Carl Dong
b396467053 locks: Annotate CTxMemPool::check to require cs_main
Currently, CTxMemPool::check locks CTxMemPool's own cs member, then
calls GetSpendHeight which locks cs_main. This can potentially cause an
undesirable lock invesion since CTxMemPool's cs is supposed to be locked
after cs_main.

This does not cause us any problems right now because all callers of
CTxMemPool already lock cs_main before calling CTxMemPool::check, which
means that the LOCK(cs_main) in GetSpendHeight becomes benign.

However, it is currently possible for new code to be added which calls
CTxMemPool::check without locking cs_main (which would be dangerous).
Therefore we should make it explicit that cs_main needs to be held
before calling CTxMemPool::check.

NOTE: After all review-only assertions are removed in "#20158 |
      tree-wide: De-globalize ChainstateManager", and assuming that we
      keep the changes in "validation: Pass in spendheight to
      CTxMemPool::check", we can re-evaluate to see if this annotation
      is still necessary.
2021-01-20 16:15:03 -05:00
Samuel Dobson
80486e7e2d Merge #20952: wallet: Add BerkeleyDB version sanity check at init time
ad57fb756b wallet: Add BerkeleyDB version sanity check at init time (Wladimir J. van der Laan)

Pull request description:

  Detect version conflicts between the run-time BerkeleyDB library and the one used during compilation.

  This is very unsafe (can result in anything from crashes to corruption) so shut down when one is detected.

ACKs for top commit:
  decryp2kanon:
    utACK ad57fb7
  achow101:
    ACK ad57fb756b
  theStack:
    utACK ad57fb756b
  meshcollider:
    Code review ACK ad57fb756b

Tree-SHA512: 99cd7d836bffbdeb3d4e14053f7139cc85a6d42e631a3f9a3058a848042446b364faee127500f5acb374616e6a61ab2bedebfac1ba9bc993b4d6227114c2a6c2
2021-01-20 16:51:42 +13:00
Wladimir J. van der Laan
bc51b99bd5 Merge #20891: rpc: Remove deprecated bumpfee behavior
ea0a7ec949 Remove deprecated bumpfee behavior (Andrew Chow)

Pull request description:

  Removes the deprecation message, behavior, and test.

  This was marked for removal in 22.0.

ACKs for top commit:
  promag:
    ACK ea0a7ec949, maybe add need release notes tag.

Tree-SHA512: d1626906849f6ee37213c32e5f8c1433ad8fb7beabcd88f5801b1964b322171a2341bdfbd9a3a5ab39b2fd9d9c6a05f73298583423a73cab1275653105c03e8e
2021-01-19 17:33:18 +01:00
HAOYUatHZ
40f05647ee doc: Add developer documentation to isminetype 2021-01-19 19:04:45 +08:00
John Newbery
bf100f8170 [net] Cleanup InactivityChecks() and add commenting about time
Also clean up and better comment the function. InactivityChecks() uses a
mixture of (non-mockable) system time and mockable time. Make sure
that's well documented.

Despite being marked as const in CConnman before this commit, the
function did mutate the state of the passed in CNode, which is contained
in vNodes, which is a member of CConnman. To make the function truly
const in CConnman and all its data, instead make InactivityChecks() a
pure function, return whether the peer should be disconnected, and let
the calling function (SocketHandler()) update the CNode object. Also
make the CNode& argument const.
2021-01-19 10:50:36 +00:00
Wladimir J. van der Laan
43f3ada27b Merge #19866: eBPF Linux tracepoints
22eb7930a6 tracing: add tracing framework (William Casarin)
933ab8a720 build: detect sys/sdt.h for eBPF tracing (William Casarin)

Pull request description:

  Instead of writing ad-hoc logging everywhere (eg: #19509), we can take advantage of linux user static defined traces, aka. USDTs ( not the stablecoin 😅 )

  The linux kernel can hook into these tracepoints at runtime, but otherwise they have little to no performance impact. Traces can pass data which can be printed externally via tools such as bpftrace. For example, here's one that prints incoming and outgoing network messages:

  # Examples

  ## Network Messages

  ```
  #!/usr/bin/env bpftrace

  BEGIN
  {
    printf("bitcoin net msgs\n");
    @start = nsecs;
  }

  usdt:./src/bitcoind:net:push_message
  {
    $ip = str(arg0);
    $peer_id = (int64)arg1;
    $command = str(arg2);
    $data_len = arg3;
    $data = buf(arg3,arg4);
    $t = (nsecs - @start) / 100000;

    printf("%zu outbound %s %s %zu %d %r\n", $t, $command, $ip, $peer_id, $data_len, $data);

    @outbound[$command]++;
  }

  usdt:./src/bitcoind:net:process_message
  {
    $ip = str(arg0);
    $peer_id = (int64)arg1;
    $command = str(arg2);
    $data_len = arg3;
    $data = buf(arg3,arg4);
    $t = (nsecs - @start) / 100000;

    printf("%zu inbound %s %s %zu %d %r\n", $t, $command, $ip, $peer_id, $data_len, $data);

    @inbound[$ip, $command]++;
  }

  ```

      $ sudo bpftrace netmsg.bt

  output: https://jb55.com/s/b11312484b601fb3.txt

  if you look at the bottom of the output you can see a histogram of all the messages grouped by message type and IP. nice!

  ## IBD Benchmarking

  ```
  #!/usr/bin/env bpftrace
  BEGIN
  {
    printf("IBD to 500,000 bench\n");
  }

  usdt:./src/bitcoind:CChainState:ConnectBlock
  {
    $height = (uint32)arg0;

    if ($height == 1) {
      printf("block 1 found, starting benchmark\n");
      @start = nsecs;
    }

    if ($height >= 500000) {
      @end = nsecs;
      @duration = @end - @start;
      exit();
    }
  }

  END {
    printf("duration %d ms\n", @duration / 1000000)
  }
  ```
  This one hooks into ConnectBlock and prints the IBD time to height 500,000 starting from the first call to ConnectBlock

  Userspace static tracepoints give lots of flexibility without invasive logging code. It's also more flexible than ad-hoc logging code, allowing you to instrument many different aspects of the system without having to enable per-subsystem logging.

  Other ideas: tracepoints for lock contention, threads, what else?

  Let me know what ya'll think and if this is worth adding to bitcoin.

  ## TODO

  - [ ] docs?
  - [x] Integrate systemtap-std-dev/libsystemtap into build (provides the <sys/sdt.h> header)
  - [x] ~dtrace macos support? (is this still a thing?)~ going to focus on linux for now

ACKs for top commit:
  laanwj:
    Tested ACK 22eb7930a6
  0xB10C:
    Tested ACK 22eb7930a6

Tree-SHA512: 69242242112b679c8a12a22b3bc50252c305894fb3055ae6e13d5f56221d858e58af1d698af55e23b69bdb7abedb5565ac6b45fa5144087b77a17acd04646a75
2021-01-18 22:09:05 +01:00
Wladimir J. van der Laan
c763cacb88 Merge #20938: build: fix linking against -latomic when building for riscv
54ce4fac80 build: improve macro for testing -latomic requirement (fanquake)
2c010b9c56 add std::atomic include to bitcoin-util.cpp (fanquake)

Pull request description:

  Since the merge of #19937, riscv builds have been failing, due to a link issue with [`std::atomic_exchange`](https://en.cppreference.com/w/cpp/atomic/atomic_exchange) in `bitcoin-util`:
  ```bash
    CXXLD    bitcoin-util
  bitcoin_util-bitcoin-util.o: In function `grind_task':
  /home/ubuntu/build/bitcoin/distsrc-riscv64-linux-gnu/src/bitcoin-util.cpp:98: undefined reference to `__atomic_exchange_1'
  collect2: error: ld returned 1 exit status

  ```

  We have a [macro](https://github.com/bitcoin/bitcoin/blob/master/build-aux/m4/l_atomic.m4) that tries to determine when `-latomic` is required, however it doesn't quite work well enough, as it's currently determining it isn't needed:
  ```bash
  ./autogen.sh
  ./configure --prefix=/home/ubuntu/bitcoin/depends/riscv64-linux-gnu
  ...
  checking whether std::atomic can be used without link library... yes
  ```

  This PR adds a call to `std::atomic_exchange` to the macro, which will get us properly linked against `-latomic` on riscv:
  ```bash
  checking whether std::atomic can be used without link library... no
  checking whether std::atomic needs -latomic... yes
  ```

  Also adds an `<atomic>` include to `bitcoin-util.cpp`.

ACKs for top commit:
  laanwj:
    Tested ACK 54ce4fac80

Tree-SHA512: 963c875097ee96b131163ae8109bcf8fecf4451d20faa2f3d223f9938ea3d8d1ed5604e12ad82c2b4b1c605fd293a9b6b08fefc00dd3e68d09c49e95029c6f50
2021-01-18 18:33:24 +01:00
Wladimir J. van der Laan
7acda55c4f Merge #20939: build: fix RELOC_SECTION security check for bitcoin-util
c061800bb1 build: fix RELOC_SECTION security check for bitcoin-util (fanquake)

Pull request description:

  The binutils we use for gitian builds strips the reloc section from
  Windows binaries, which breaks ASLR. As a temporary workaround, export
  main(). This is the same workaround as #18702 (bitcoin-cli), and will
  fix the currently failing security check:
  ```bash
  + make -j1 -C src check-security
  make: Entering directory '/home/ubuntu/build/bitcoin/distsrc-x86_64-w64-mingw32/src'
  Checking binary security...
  bitcoin-util.exe: failed RELOC_SECTION
  make: *** [check-security] Error 1
  ```

  Relevant upstream issue:
  https://sourceware.org/bugzilla/show_bug.cgi?id=19011

ACKs for top commit:
  dongcarl:
    ACK c061800bb1
  laanwj:
    ACK c061800bb1

Tree-SHA512: a1a4da0b2cddfc377190b9044a04f42a859ca79f11ce2c2ab4c3d066a2786c34d5446d75f8eec634f308d2d3349ebbd7c9f76dcaebeeb28e471c829851592367
2021-01-17 18:12:08 +01:00
Wladimir J. van der Laan
ad57fb756b wallet: Add BerkeleyDB version sanity check at init time
Detect version conflicts between the run-time BerkeleyDB library and the one used during compilation.

This is very unsafe (can result in anything from crashes to corruption) so shut down when one is detected.
2021-01-17 18:10:20 +01:00
Kiminuo
da9caa1ced Replace fs::absolute calls with AbsPathJoin calls
This adds better test coverage and will make it easier in #20744 to remove our dependency on the two-argument boost::filesystem::absolute() function which does not have a direct equivalent in C++17.
2021-01-15 22:48:15 +01:00
Kiminuo
66576c4fd5 test: Clear forced -walletdir setting after wallet init_tests
Leaving this value set interfered with the CreateWallet test if it happened to execute later in the test ordering. Specifically it would cause CreateWallet test to write data to the current directory instead of temporary test directory.
2021-01-15 20:19:31 +01:00
MarcoFalke
32e59fc371 Merge #20916: rpc: Return wtxid from testmempoolaccept
fa0aa87071 rpc: Return wtxid from testmempoolaccept (MarcoFalke)

Pull request description:

  It would be nice if `testmempoolaccept` returned the unique wtxid directly to avoid a costly `decoderawtransaction` roundtrip

ACKs for top commit:
  mjdietzx:
    utACK fa0aa87071
  stackman27:
    utACK [`fa0aa87`](fa0aa87071)
  glozow:
    cr ACK fa0aa87071

Tree-SHA512: 05dbaf46d93e47e9eedb725c2f57175e6d4e1722da0531fe4f80e74fc2518911da87634f25f61fa2bc8d87a3017e82fd0684b09a0a9706d71deed970035c2e7a
2021-01-15 20:13:34 +01:00
MarcoFalke
0a1cf6c347 Merge #20908: fuzz: Use mocktime in process_message* fuzz targets
fa0a864b38 fuzz: Use mocktime in process_message* fuzz targets (MarcoFalke)

Pull request description:

  Use mocktime to allow time to advance deterministically during execution of a fuzz input. This also allows to drop the call to `JumpOutOfIbd`.

ACKs for top commit:
  practicalswift:
    cr ACK fa0a864b38

Tree-SHA512: e92fc70ec6bd49760173cb202549f364304e22b3f7127b9a4da8447cf9341008e477ad42c2599c2fde167bbcbc0e2d139709b4ef6371788bc2c1c3b7f589e11d
2021-01-15 19:56:18 +01:00
fanquake
c061800bb1 build: fix RELOC_SECTION security check for bitcoin-util
The binutils we use for gitian builds strips the reloc section from
Windows binaries, which breaks ASLR. As a temporary workaround, export
main(). This is the same workaround as #18702 (bitcoin-cli), and will
fix the currently failing security check:
```bash
+ make -j1 -C src check-security
make: Entering directory '/home/ubuntu/build/bitcoin/distsrc-x86_64-w64-mingw32/src'
Checking binary security...
bitcoin-util.exe: failed RELOC_SECTION
make: *** [check-security] Error 1
```

Relevant upstream issue:
https://sourceware.org/bugzilla/show_bug.cgi?id=19011
2021-01-15 11:53:14 +08:00
fanquake
2c010b9c56 add std::atomic include to bitcoin-util.cpp 2021-01-15 10:40:29 +08:00
fanquake
f91587f050 Merge #20834: locks and docs in ATMP and CheckInputsFromMempoolAndCache
2f463f57e3 [doc] for CheckInputsFromMempoolAndCache (gzhao408)
85cc6bed64 lock annotations for MemPoolAccept functions (gzhao408)

Pull request description:

  This is a very small PR that adds some lock annotations to clarify that, now, the `pool.cs` lock is held throughout tx validation for mempool.  The comments in `CheckInputsFromMempoolAndCache` were unclear/outdated so I updated those as well.

  ~This PR is a cleanup. It removes unnecessary code that doesn't do much.~

ACKs for top commit:
  sdaftuar:
    utACK 2f463f57e3
  jnewbery:
    utACK 2f463f57e3
  MarcoFalke:
    cr ACK 2f463f57e3
  fanquake:
    ACK 2f463f57e3

Tree-SHA512: 5863a546b00ef02eba8f208c2c04c32f64671f17c967a5e3c51332fc0f472e5e9addddae075d0b91c77caebe1be9331317646b0bec802e86d9550773fd9621a9
2021-01-14 23:35:41 +08:00
MarcoFalke
29d2aeb4a2 Merge #20828: fuzz: Introduce CallOneOf helper to replace switch-case
fa75d40ef8 fuzz: Introduce CallOneOf helper to replace switch-case (MarcoFalke)

Pull request description:

  The current `switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, nn)) { case 0: ... case 1: ... case nn: ...` has several problems:

  * It makes it hard to review newly added targets, because it requires manual counting of cases
  * It makes it hard to update a target, because updating all case labels is trivial, but tedious to review and causes merge conflicts
  * ~~Updating the target raises the question whether the case labels should be preserved to not invalidate the existing fuzz inputs format. Fuzz input format might already change implicitly on every commit, so this isn't something worthwhile to pursue.~~ Edit: This pull doesn't fix this problem.

  Fix all issues by adding a new `CallOneOf` helper

ACKs for top commit:
  ajtowns:
    ACK fa75d40ef8 - code review only
  jnewbery:
    utACK fa75d40ef8

Tree-SHA512: 2daa602b240b86c8e85a024e008f03a57ba60349377eed771f4d21a97a9dba9b66e93fff16ff1992018d4330be7a1a276944c3dfdf698748ce135626c380e563
2021-01-14 11:07:22 +01:00
MarcoFalke
cb2c578451 Merge bitcoin-core/gui#148: Bugfix: GUI: Restore SendConfirmationDialog button default to "Yes"
8775691383 Bugfix: GUI: Restore SendConfirmationDialog button default to "Yes" (Luke Dashjr)

Pull request description:

  The SendConfirmationDialog is used for bumping the fee, where "Send" doesn't really make sense

  Originally https://github.com/bitcoin/bitcoin/pull/17463, but rewritten here much simpler based on other merged changes.

ACKs for top commit:
  hebasto:
    ACK 8775691383, tested on Linux Mint 20.1 (x86_64, Qt 5.12.8):

Tree-SHA512: 3953cc9c09613c9a629def8b4dc061b537f148ddcb378430645602e0be0f3a9f1cff083aa685b94b2e9372300d02ec97e0d9ea89db6e3c6feec86795090f0f77
2021-01-13 17:47:46 +01:00
MarcoFalke
22fa9673b0 Merge #20917: doc, rpc: add missing signet mentions in network name lists
fc726e0138 doc, rpc: add missing signet mentions in network name lists (Sebastian Falbesoner)

Pull request description:

  This small PR adds a few missing mentions of signet w.r.t. chain enumerations:

  - RPC `getblockchaininfo`: result description for `"chain"`
  - RPC `getmininginfo`: result description for `"chain"`
  - REST interface documentation:
      - default ports listing for each chain
      - `"chain"` description for `chaininfo` endpoint result

  The instances were identified via `git grep -i "main.*test.*reg"`.

ACKs for top commit:
  ajtowns:
    ACK fc726e0138 -- quick code review only
  benthecarman:
    ACK fc726e0138

Tree-SHA512: 62cdc6ef74fa10db75cc04b9eaf7367183f726b3fee3d21fdf741b3816669dd21508735e89da389ddac980f49773ab229263748d1399553375fefe4526361846
2021-01-13 17:31:47 +01:00
John Newbery
06fa85cd50 [net] InactivityCheck() takes a CNode reference 2021-01-13 16:18:12 +00:00
Wladimir J. van der Laan
e7eb37128c Merge #20913: doc: Add manual page generation for bitcoin-util
bc99ae77e4 scripted-diff: Fix typo in stub manual pages (Wladimir J. van der Laan)
b5e93f873a doc: Add manual page generation for bitcoin-util (Wladimir J. van der Laan)

Pull request description:

  - Add `-version` option to `bitcoin-util`
  - Add `bitcoin-util` call to `gen-manpages.sh`
  - Add stub manual page `bitcoin-util.1`
  - Add install of `bitcoin-util.1` to build system

ACKs for top commit:
  fanquake:
    ACK bc99ae77e4

Tree-SHA512: 948df66c62bbca1cf6da26845dfa63f8f5d036a3d5744add468dd1ce7f442c123d7b0db7011c2e8e3ee6539fd391c7ee2c21b706ec81b21b02821c9501cd077d
2021-01-13 10:16:09 +01:00
Wladimir J. van der Laan
60427ee35f Merge #20811: refactor: move net_processing implementation details out of header
c97f70c861 net_processing: move Peer definition to .cpp (Anthony Towns)
e0f2e6d2df net_processing: move PeerManagerImpl into cpp file (Anthony Towns)
a568b82feb net_processing: split PeerManager into interface and implementation classes (Anthony Towns)
0df3d3fd6b net_processing: make more of PeerManager private (Anthony Towns)
0d246a59b6 net, net_processing: move NetEventsInterface method docs to net.h (Anthony Towns)

Pull request description:

  Moves the implementation details of `PeerManager` and all of `struct Peer` into net_processing.cpp.

ACKs for top commit:
  jnewbery:
    ACK c97f70c861
  Sjors:
    ACK c97f70c861
  laanwj:
    Code review ACK c97f70c861
  vasild:
    ACK c97f70c861

Tree-SHA512: 2e081e491d981c61bd78436a6a6c2eb41d3c2d86a1a8ef9d4d6f801b82cb64a8bf707a96db3429418d1704cffb60a657d1037a0336cbc8173fb79aef9eb72001
2021-01-13 09:48:06 +01:00
Wladimir J. van der Laan
8ffaf5c2f5 Merge #19935: Move SaltedHashers to separate file and add some new ones
281fd1a4a0 Replace KeyIDHasher with SaltedSipHasher (Andrew Chow)
210b693db6 Add generic SaltedSipHasher (Andrew Chow)
95e61c1cf2 Move Hashers to util/hasher.{cpp/h} (Andrew Chow)

Pull request description:

  There are existing `SaltedOutPointHasher` and `SaltedTxidHasher` classes used for `std::unordered_map` and `std::unordered_set` that could be useful in other places in the codebase. So we these to their own `saltedhash.{cpp/h}` file. An existing `KeyIDHasher` is moved there too. Additionally, `ScriptIDHasher`, `SaltedPubkeyHasher`, and `SaltedScriptHasher` are added so that they can be used in future work.

  `KeyIDHasher` and `ScriptIDHasher` are not salted so that equality comparisons of maps and sets keyed by `CKeyID` and `CScriptID` will actually work.

  Split from #19602 (and a few other PRs/branches I have).

ACKs for top commit:
  laanwj:
    Code review ACK 281fd1a4a0
  jonatack:
    ACK 281fd1a4a0, code review, debug build and ran bitcoind after rebasing to master @ dff0f6f753
  fjahr:
    utACK 281fd1a4a0

Tree-SHA512: bb03b231ccf3c9ecefc997b8da9c3770af4819f9be5b0a72997a103864e84046a2ac39b8eadf0dc9247bdccd53f86f433642e3a098882e6748341a9e7736271b
2021-01-13 08:49:17 +01:00
MarcoFalke
fa0a864b38 fuzz: Use mocktime in process_message* fuzz targets 2021-01-13 07:48:41 +01:00
MarcoFalke
fa0aa87071 rpc: Return wtxid from testmempoolaccept 2021-01-12 18:43:43 +01:00
Sebastian Falbesoner
fc726e0138 doc, rpc: add missing signet mentions in network name lists 2021-01-12 18:43:31 +01:00
Wladimir J. van der Laan
b5e93f873a doc: Add manual page generation for bitcoin-util
- Add `-version` option to `bitcoin-util`
- Add `bitcoin-util` call to `gen-manpages.sh`
- Add stub manual page `bitcoin-util.1`
- Add install of `bitcoin-util.1` to build system
2021-01-12 14:09:21 +01:00
Wladimir J. van der Laan
7b975639ef Merge #19937: signet mining utility
595a34dbea contrib/signet: Document miner script in README.md (Anthony Towns)
ff7dbdc08a contrib/signet: Add script for generating a signet chain (Anthony Towns)
13762bcc96 Add bitcoin-util command line utility (Anthony Towns)
95d5d5e625 rpc: allow getblocktemplate for test chains when unconnected or in IBD (Anthony Towns)
81c54dec20 rpc: update getblocktemplate with signet rule, include signet_challenge (Anthony Towns)

Pull request description:

  Adds `contrib/signet/miner` for mining signet blocks.

  Adds `bitcoin-util` cli utility, with the idea being it can provide bitcoin related functionality that does not rely on the ability to access a running node. Only subcommand currently is "grind" which takes a hex-encoded header and grinds its nonce until its nBits is satisfied.

  Updates `getblocktemplate` to include `signet_challenge` field, and makes `getblocktemplate` require the signet rule when invoked on the signet change. Removes connectivity and IBD checks from `getblocktemplate` when applied to a test chain (regtest, testnet, signet).

ACKs for top commit:
  laanwj:
    code review ACK 595a34dbea

Tree-SHA512: 8d43297710fdc1edc58acd9b53e1bd1671e5724f7097b40ab73653715dc8becc70534c4496cbba9290f4dd6538a7a3d5830eb85f83391ea31a3bb5b9d3378cc3
2021-01-12 12:53:45 +01:00
gzhao408
2f463f57e3 [doc] for CheckInputsFromMempoolAndCache 2021-01-12 02:27:09 -08:00
gzhao408
85cc6bed64 lock annotations for MemPoolAccept functions
We should already have the mempool lock when entering
CheckInputsFromMempoolAndCache
2021-01-12 02:27:09 -08:00