Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

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

Remove leftover bumpfee code (RBF) #5

Merged

Conversation

ftrader
Copy link
Contributor

@ftrader ftrader commented May 11, 2017

This removes the leftover 'bumpfee' (RBF) regtest and client code.

There is an uncertainty in the removed wallet code - one check that depends on a 'replaced_by_txid' count has been left in as that count is still calculated. This needs careful review.

@ftrader
Copy link
Contributor Author

ftrader commented May 11, 2017

Once we have reviewed the change to src/wallet/wallet.cpp and are sure about it, I can remove the extra comments there and squash.

@deadalnix deadalnix merged commit 8350392 into Bitcoin-ABC:master May 11, 2017
@ftrader ftrader deleted the patch/remove_bumpfee branch May 11, 2017 10:57
cculianu pushed a commit to cculianu/bitcoin-abc that referenced this pull request Oct 5, 2017
Summary:
Includes:

  bitcoin-core/leveldb-subtree#2: Prefer std::atomic over MemoryBarrier (Pieter Wuille)
  bitcoin-core/leveldb-subtree#5: Move helper functions out of sse4.2 object (Cory Fields)
  bitcoin-core/leveldb-subtree#6: Fixes typo (Dimitris Tsapakidis)
  bitcoin-core/leveldb-subtree#10: Clean up compile-time warnings (gcc 7.1) (Matt Corallo)
  bitcoin-core/leveldb-subtree#11: fixup define checks. Cleans up some oopses from Bitcoin-ABC#5 (Cory Fields)

Removes many warnings on MacOSX of the form:

  In file included from leveldb/util/cache.cc:10:
  In file included from ./leveldb/port/port.h:14:
  In file included from ./leveldb/port/port_posix.h:47:
  ./leveldb/port/atomic_pointer.h:55:3: warning: 'OSMemoryBarrier' is deprecated: first deprecated in macOS 10.12 - Use
      std::atomic_thread_fence() from <atomic> instead [-Wdeprecated-declarations]
        OSMemoryBarrier();
        ^

The bitcoin core patches have been included in BU already by sickpig.

Test Plan: make check.  Installed and running fine.

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D584
lateminer pushed a commit to lateminer/bitcoin-abc that referenced this pull request Apr 1, 2018
deadalnix pushed a commit that referenced this pull request Jun 26, 2020
Summary:
We see occasional flakiness on TSAN that gives not-so-useful output like this:
```
==5540==WARNING: failed to fork (errno 12)
==5540==WARNING: Failed to use and restart external symbolizer!
==================
WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=5540)
  Cycle in lock order graph: M100059481 (0x7fffa93419f8) => M100059482 (0x7fffa93419d0) => M100059481

  Mutex M100059482 acquired here while holding mutex M100059481 in main thread:
    #0 <null> <null> (test_bitcoin+0xeacce)
    #1 <null> <null> (test_bitcoin+0x7b9675)
    #2 <null> <null> (test_bitcoin+0x7b8e8b)
    #3 <null> <null> (test_bitcoin+0x198299)
    #4 <null> <null> (libboost_unit_test_framework.so.1.67.0+0x55dbd)
    #5 <null> <null> (libc.so.6+0x2409a)
```

Attempts to symbolize that memory manually suggests a deadlock that is already in our suppressions file.

Although clang sanitizers will default to llvm-symbolizer, it's not in our path, so it defaults to addr2line.
It's not clear if addr2line can be assumed to perform as well as llvm-symbolizer. Testing both locally
indicates they produce very similar output when suppressions files are not used, for instance.

This patch is more of an experiment to see if llvm-symbolizer is able to correctly symbolize the above logs.
If so, either the suppression will stop being flaky or we will obtain useful information to pinpoint the root
cause.

Test Plan: Run ASAN, TSAN, UBSAN on CI and make sure they still pass

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D6746
deadalnix pushed a commit that referenced this pull request Dec 28, 2020
…Travis

Summary:
1f9d5af4f197e7cc0469a0bb25dcbc51dfa537f4 tests: Add initialization order fiasco detection in Travis (practicalswift)

Pull request description:

  Add initialization order fiasco detection in Travis :)

  Context: bitcoin/bitcoin#17670 (comment)

  This would have caught the `events_hasher` initialization order issue introduced in #17573  and fixed in #17670.

  Output in case of an initialization order fiasco:

  ```
  ==7934==ERROR: AddressSanitizer: initialization-order-fiasco on address 0x557098d79200 at pc 0x55709796b9a3 bp 0x7ffde524dc30 sp 0x7ffde524dc28
  READ of size 8 at 0x557098d79200 thread T0
      #0 0x55709796b9a2 in CSHA256::Finalize(unsigned char*) src/crypto/sha256.cpp:667:25
      #1 0x5570978150e9 in SeedEvents(CSHA512&) src/random.cpp:462:19
      #2 0x5570978145e1 in SeedSlow(CSHA512&) src/random.cpp:482:5
      #3 0x5570978149a3 in SeedStartup(CSHA512&, (anonymous namespace)::RNGState&) src/random.cpp:527:5
      #4 0x55709781102d in ProcRand(unsigned char*, int, RNGLevel) src/random.cpp:571:9
      #5 0x557097810d19 in GetRandBytes(unsigned char*, int) src/random.cpp:576:59
      #6 0x557096c2f9d5 in (anonymous namespace)::CSignatureCache::CSignatureCache() src/script/sigcache.cpp:34:9
      #7 0x557096511977 in __cxx_global_var_init.7 src/script/sigcache.cpp:67:24
      #8 0x5570965119f8 in _GLOBAL__sub_I_sigcache.cpp src/script/sigcache.cpp
      #9 0x557097bba4ac in __libc_csu_init (src/bitcoind+0x18554ac)
      #10 0x7f214b1c2b27 in __libc_start_main /build/glibc-OTsEL5/glibc-2.27/csu/../csu/libc-start.c:266
      #11 0x5570965347d9 in _start (src/bitcoind+0x1cf7d9)

  0x557098d79200 is located 96 bytes inside of global variable 'events_hasher' defined in 'random.cpp:456:16' (0x557098d791a0) of size 104
    registered at:
      #0 0x557096545dfd in __asan_register_globals compiler-rt/lib/asan/asan_globals.cpp:360:3
      #1 0x557097817f8b in asan.module_ctor (src/bitcoind+0x14b2f8b)

  SUMMARY: AddressSanitizer: initialization-order-fiasco src/crypto/sha256.cpp:667:25 in CSHA256::Finalize(unsigned char*)
  ```

---

Backport of Core [[bitcoin/bitcoin#17674 | PR17674]]

Depends on D8756

Test Plan:
  cmake -GNinja -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DENABLE_SANITIZERS=address
  ninja all check check-functional

Reviewers: #bitcoin_abc, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Differential Revision: https://reviews.bitcoinabc.org/D8757
Fabcien pushed a commit that referenced this pull request Oct 13, 2021
Summary:
See https://build.bitcoinabc.org/viewLog.html?buildId=300891&guest=1 for a tsan race while running `interface_zmq.py`
```
 node0 stdout ==================
WARNING: ThreadSanitizer: data race (pid=10367)
  Read of size 8 at 0x7b2400030238 by thread T19:
    #0 memcpy <null> (bitcoind+0x2645a7)
    #1 <null> <null> (libzmq.so.5+0x76220)

  Previous write of size 8 at 0x7b2400030238 by thread T12:
    #0 malloc <null> (bitcoind+0x258ee4)
    #1 <null> <null> (libzmq.so.5+0x39578)
    #2 CZMQAbstractPublishNotifier::SendZmqMessage(char const*, void const*, unsigned long) /work/abc-ci-builds/build-tsan/../../src/zmq/zmqpublishnotifier.cpp:164:14 (bitcoind+0xa1e7a9)
    ...
0
  Location is heap block of size 140 at 0x7b2400030210 allocated by thread T12:
    #0 malloc <null> (bitcoind+0x258ee4)
    #1 <null> <null> (libzmq.so.5+0x39578)
    #2 CZMQAbstractPublishNotifier::SendZmqMessage(char const*, void const*, unsigned long) /work/abc-ci-builds/build-tsan/../../src/zmq/zmqpublishnotifier.cpp:164:14 (bitcoind+0xa1e7a9)
    ...

  Thread T19 'ZMQbg/1' (tid=10388, running) created by main thread at:
    #0 pthread_create <null> (bitcoind+0x25a7ab)
    #1 <null> <null> (libzmq.so.5+0x6e8b3)
    #2 CZMQNotificationInterface::Initialize() /work/abc-ci-builds/build-tsan/../../src/zmq/zmqnotificationinterface.cpp:86:23 (bitcoind+0xa199d4)
    #3 CZMQNotificationInterface::Create() /work/abc-ci-builds/build-tsan/../../src/zmq/zmqnotificationinterface.cpp:61:36 (bitcoind+0xa19328)
    #4 AppInitMain(Config&, RPCServer&, HTTPRPCRequestProcessor&, NodeContext&, interfaces::BlockAndHeaderTipInfo*) /work/abc-ci-builds/build-tsan/../../src/init.cpp:2482:36 (bitcoind+0x30d673)
    #5 AppInit(int, char**) /work/abc-ci-builds/build-tsan/../../src/bitcoind.cpp:181:16 (bitcoind+0x2eaa8e)
    #6 main /work/abc-ci-builds/build-tsan/../../src/bitcoind.cpp:208:13 (bitcoind+0x2eaa8e)

  Thread T12 'b-scheduler' (tid=10380, running) created by main thread at:
    #0 pthread_create <null> (bitcoind+0x25a7ab)
    #1 boost::thread::start_thread_noexcept() <null> (libboost_thread.so.1.67.0+0x1396a)
    #2 boost::thread::thread<std::_Bind<void (* (char const*, std::function<void ()>))(char const*, std::function<void ()>)>&>(std::_Bind<void (* (char const*, std::function<void ()>))(char const*, std::function<void ()>)>&) /usr/include/boost/thread/detail/thread.hpp:266:13 (bitcoind+0x330342)
    #3 boost::thread* boost::thread_group::create_thread<std::_Bind<void (* (char const*, std::function<void ()>))(char const*, std::function<void ()>)> >(std::_Bind<void (* (char const*, std::function<void ()>))(char const*, std::function<void ()>)>) /usr/include/boost/thread/detail/thread_group.hpp:79:60 (bitcoind+0x32039e)
    #4 AppInitMain(Config&, RPCServer&, HTTPRPCRequestProcessor&, NodeContext&, interfaces::BlockAndHeaderTipInfo*) /work/abc-ci-builds/build-tsan/../../src/init.cpp:2243:17 (bitcoind+0x309628)
    #5 AppInit(int, char**) /work/abc-ci-builds/build-tsan/../../src/bitcoind.cpp:181:16 (bitcoind+0x2eaa8e)
    #6 main /work/abc-ci-builds/build-tsan/../../src/bitcoind.cpp:208:13 (bitcoind+0x2eaa8e)

    #1 <null> <null> (libzmq.so.5+0x6e8b3)
    #2 CZMQNotificationInterface::Initialize() /work/abc-ci-builds/build-tsan/../../src/zmq/zmqnotificationinterface.cpp:86:23 (bitcoind+0xa199d4)
    #3 CZMQNotificationInterface::Create() /work/abc-ci-builds/build-tsan/../../src/zmq/zmqnotificationinterface.cpp:61:36 (bitcoind+0xa19328)
    #4 AppInitMain(Config&, RPCServer&, HTTPRPCRequestProcessor&, NodeContext&, interfaces::BlockAndHeaderTipInfo*) /work/abc-ci-builds/build-tsan/../../src/init.cpp:2482:36 (bitcoind+0x30d673)
    #5 AppInit(int, char**) /work/abc-ci-builds/build-tsan/../../src/bitcoind.cpp:181:16 (bitcoind+0x2eaa8e)
```

This is a backport of [[bitcoin/bitcoin#20748 | core#20748]]

Test Plan:
With TSAN:
`for i in $(seq 100); do TSAN_OPTIONS=second_deadlock_stack=1: TSAN_OPTIONS=suppressions=/home/pierre/dev/bitcoin-abc/test/sanitizer_suppressions/tsan test/functional/test_runner.py interface_zmq; done`

This failure is intermittent, but relatively easy to reproduce: I was able to get it with 10 repetitions of the test, prior to adding the suppression.

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D10315
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants