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 SegWit regression tests (Python) #1

Conversation

ftrader
Copy link
Contributor

@ftrader ftrader commented May 7, 2017

These tests require SegWit functionality, and need to be removed:

  • segwit.py
  • p2p-segwit.py

These tests require SegWit functionality, and need to be removed.
@deadalnix
Copy link
Member

Travis is failing for some reason.

@ftrader
Copy link
Contributor Author

ftrader commented May 7, 2017

It's failing because nearly every other test fails with

Warning: Error reading wallet.dat! All keys read correctly, but transaction data or address book entries might be missing or incorrect.

This is some other problem unrelated to this commit. It's probably worth fixing that first in a separate PR, then rebasing this PR and trying again.

There are probably more problems lurking.

@deadalnix deadalnix merged commit 5ce7890 into Bitcoin-ABC:master May 7, 2017
@ftrader ftrader deleted the patch/remove_segwit_regtests branch May 10, 2017 14:43
jasonbcox pushed a commit that referenced this pull request Oct 11, 2019
…split keys after upgrading to hd chain split

Summary:
(commit #1)
Changes the maximum upgradewallet version to the latest wallet version
number, 190700. Non-HD wallets will be upgraded to use HD derivation.
Non HD chain split wallets will be upgraded to HD chain split.

(commit #2)
After upgrading to HD chain split, we want to continue to use keys
from the old keypool. To do this, before we generate any new keys after
upgrading, we mark all of the keypool entries as being pre-chain
split and move them to a separate pre chain split keypool. Keys are
fetched from that keypool until it is emptied. Only then are the new
internal and external keypools used.

Since upgradewallet is effectively run during a first run, all of the
first run initial setup stuff is combined with the upgrade to HD

Partial backport of Core PR12560 (3/4)
bitcoin/bitcoin@5c50e93
bitcoin/bitcoin@dfcd9f3

Depends on D4202

Test Plan:
Recommend to perform testing on `regtest` or else this process may take a very long time.
  make check
  ./bitcoind -upgradewallet
  ./bitcoin-cli getwalletinfo > preupgradewalletinfo
  ./bitcoin-cli generate 1
  Kill bitcoind
  ./bitcoind -printtoconsole -upgradewallet
bitcoind should output to console the two following lines:
  Performing wallet upgrade to 190700
  Upgrading wallet to use HD chain split
Compare previous `preupgradewalletinfo` with `./bitcoin-cli getwalletinfo` post upgrade

preupgradewalletinfo:
  {
    "walletname": "",
    "walletversion": 160300,
    "balance": 0.00000000,
    "unconfirmed_balance": 0.00000000,
    "immature_balance": 0.00000000,
    "txcount": 0,
    "keypoololdest": 1551725443,
    "keypoolsize": 1000,
    "keypoolsize_hd_internal": 1000,
    "paytxfee": 0.00000000,
    "hdmasterkeyid": "c13ec93790a56f74edd2da42d231317eb6dbf02b"
  }

post upgrade wallet info:
  {
    "walletname": "",
    "walletversion": 190700,
    "balance": 0.00000000,
    "unconfirmed_balance": 0.00000000,
    "immature_balance": 50.00000000,
    "txcount": 1,
    "keypoololdest": 1551725443,
    "keypoolsize": 1000,
    "keypoolsize_hd_internal": 1000,
    "paytxfee": 0.00000000,
    "hdmasterkeyid": "c13ec93790a56f74edd2da42d231317eb6dbf02b"
  }
The newly upgraded wallet should not be able to be downgraded to a previous version
  Kill bitcoind
  ./bitcoind -upgradewallet=160300
This should fail and cause `bitcoind` to shutdown with the following line:
  Error: Cannot downgrade wallet

Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D4206
Mengerian pushed a commit to Mengerian/bitcoin-abc that referenced this pull request Oct 31, 2019
Mengerian pushed a commit to Mengerian/bitcoin-abc that referenced this pull request Oct 31, 2019
deadalnix pushed a commit that referenced this pull request Jan 12, 2020
Summary:
In Bitcoin Core, virtual size has two meanings:

1. BIP 141 defines virtual size = weight/4 (rounded up), i.e. transaction size
  plus witness size divided by 4.
  (https://github.com/bitcoin/bips/blob/master/bip-0141.mediawiki#additional-definitions )

2. Virtual size = max(weight, sigopscost*bytespersigop) / 4 rounded up, used in
  all mempool code to punish very very high sigops transactions.

Currently we have no code that uses meaning #2, but we do have some weird
backported code that uses meaning #1, which is definitely inappropriate
for BCH since we don't have segwit.

This diff removes that stuff and also removes the ability to ask for
virtual size in the BIP141 convention (without a sigops count), which is always
going to be a conceptual mistake in our codebase.

(see D3655 D3316 D3180 D3243)

Test Plan: `ninja all check bench-bitcoin`

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D4895
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
…return checking

Summary:
8d22ab0e503ccaa464cbecd94d1059dbc5a61f4a 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
  …
  $
  ```

---

Backport of Core [[bitcoin/bitcoin#17205 | PR17205]]

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/D8756
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 Jul 1, 2021
Summary:
Some Bitcoin activity is completely local (offline), e.g., reindexing.

The `setnetworkactive` RPC command is already present. This PR adds the corresponding command-line argument / config option, and allows to start the client with disabled p2p network by providing `-networkactive=0` or `-nonetworkactive`.

Commit message #1:
> net: Add -networkactive option
>
> The `setnetworkactive' RPC command is already present.
> This new option allows to start the client with disabled p2p network
> activity for testing or reindexing.

Commit message #2:
>net: Log network activity status change unconditionally

Commit message #3:
> test: Add test coverage for -networkactive option

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

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D9713
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
Fabcien pushed a commit that referenced this pull request Dec 23, 2021
…malformed time field

Summary: This is a backport of [[bitcoin/bitcoin#20372 | core#20372]]

Test Plan:
This causes an error before applying the commit, and it works after the change:
```
$ cmake .. -GNinja -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DENABLE_SANITIZERS=undefined
$ xxd -p -r > mempool.dat-crash-1 <<EOF
0100000000000000000000000004000000000000000000000000ffffffff
ffffff7f00000000000000000000000000
EOF
$ cp mempool.dat-crash-1 /bitcoinddata/regtest/mempool.dat
$ ninja
$ UBSAN_OPTIONS="print_stacktrace=1:halt_on_error=1:report_error_type=1" src/bitcoind -regtest
```

```
../src/validation.cpp:5853:23: runtime error: signed integer overflow: 9223372036854775807 + 1209600 cannot be represented in type 'long'
    #0 0x55f14103ffcd in LoadMempool(Config const&, CTxMemPool&) /home/pierre/dev/bitcoin-abc/build_ubsan/../src/validation.cpp:5853:23
    #1 0x55f14103fb65 in CChainState::LoadMempool(Config const&, ArgsManager const&) /home/pierre/dev/bitcoin-abc/build_ubsan/../src/validation.cpp:4821:9
...
```

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D10725
Fabcien pushed a commit that referenced this pull request Nov 17, 2022
Summary:
This is needed to turn globals into member variables. Otherwise, this
will lead to issues:

```
runtime error: reference binding to null pointer of type 'CBlockFileInfo'
    #0 in std::vector<CBlockFileInfo, std::allocator<CBlockFileInfo> >::operator[](unsigned long) /usr/bin/../lib/gcc/x86_64-linux-gnu/11/../../../../include/c++/11/bits/stl_vector.h:1046:2
    #1 in BlockManager::FlushBlockFile(bool, bool) src/node/blockstorage.cpp:540:47
    #2 in CChainState::FlushStateToDisk(BlockValidationState&, FlushStateMode, int) src/validation.cpp:2262:28
    #3 in CChainState::ResizeCoinsCaches(unsigned long, unsigned long) src/validation.cpp:4414:15
    #4 in validation_chainstate_tests::validation_chainstate_resize_caches::test_method() src/test/validation_chainstate_tests.cpp:66:12
```
This is a partial backport of [[bitcoin/bitcoin#23974 | core#23974]]
bitcoin/bitcoin@fad381b

Depends on D12512

Test Plan: `ninja check`

Reviewers: #bitcoin_abc, sdulfari

Reviewed By: #bitcoin_abc, sdulfari

Differential Revision: https://reviews.bitcoinabc.org/D12513
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.

2 participants