Skip to content

Conversation

@jonspock
Copy link
Member

@jonspock jonspock commented Apr 6, 2019

Completion of useful update between Bitcoin-abc 19.1 and 19.2
Tested locally, can be squashed and merged or just merged

@ghost ghost assigned jonspock Apr 6, 2019
@ghost ghost added the in progress label Apr 6, 2019
MarcoFalke and others added 27 commits April 8, 2019 13:35
Summary:
a9b6ba0b7 Add missing cs_main locks when calling blockToJSON/blockheaderToJSON (practicalswift)

Pull request description:

  `blockToJSON(...)` and `blockheaderToJSON(...)` read the variable `chainActive` which requires holding the mutex `cs_main`. So does `GetDifficulty(...)`.

Tree-SHA512: bfb94f5e3238accbf6a4daddde49d53f1891c38ae9b07e25b3098c485747159258f64bb66a50e147b32beac601de89d9d04ff717b6c4f1460d329c90a53d3333

Backport of Core PR 11618
https://github.com/bitcoin/bitcoin/pull/11618/files

Test Plan:
make check
test_runner.py
bitcoin-cli getblock 000000000933ea01ad0ee984209779baaec3ced90fa3f408719526f8d77f4943

Reviewers: deadalnix, Fabien, #bitcoin_abc

Reviewed By: Fabien, #bitcoin_abc

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2684
Summary:
4dc1915 check for null values in rpc args and handle appropriately (Gregory Sanders)
999ef20 importmulti options are optional (Gregory Sanders)
a70d025 fixup some rpc param counting for rpc help (Gregory Sanders)

Pull request description:

  Audited where named args will fail to use correct default values or may fail when additional optional arguments are added.

  Previously for these parameters, it was fine to omit them as positional arguments, but it would trigger UniValue runtime errors to set them to null, or to omit them while passing named parameters with greater positions (which would internally set earlier missing arguments to null). Now null values are treated the same as missing values so these errors do not occur.

  Included a few other small fixes while working on it.

  I didn't bother fixing account-based rpc calls.

Tree-SHA512: 8baf781a35bd48de7878d4726850a580dab80323d3416c1c146b4fa9062f8a233c03f37e8ae3f3159e9d04a8f39c326627ca64c14e1cb7ce72538f934ab2ae1e

Backport of Core PR 10783
https://github.com/bitcoin/bitcoin/pull/10783/files
Completes T552

Test Plan:
make check
test_runner.py

Reviewers: deadalnix, Fabien, #bitcoin_abc

Reviewed By: Fabien, #bitcoin_abc

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2688
Summary:
d9d1bd3 nCheckDepth chain height fix (romanornr)

Pull request description:

  ````
  if (nCheckDepth <= 0)
      nCheckDepth = 1000000000; // suffices until the year 19000
  if (nCheckDepth > chainActive.Height())
      nCheckDepth = chainActive.Height();
  ````

  These lines confuse me.
  Correct me if I am wrong, but we can't check any more blocks than we have right?
  If someone requests <= 0 it get set it into some huge number and then immediately limit it to the chain height in the following statement.
  ````
  if (nCheckDepth > chainActive.Height())
      nCheckDepth = chainActive.Height();
  ````
  when using ````--checkblocks=Z```` When Z is ````0```` or any other negative number, it will check all blocks.

  I think it should be changed to this maybe.
  ````
  if (nCheckDepth <= 0 || nCheckDepth > chainActive.Height())
      nCheckDepth = chainActive.Height();
  ````
  Which gets rid of that huge number which is confusing for any other altcoins that have a different block time.

Tree-SHA512: 8ee0ae5f33b399fa74dc16926709694ccfe1fc8a043cba2f5d00884220ac1b9b13f2df4588041f4133be634e5c7b14f4eebe24294028dafe91581a97dbe627f3

Backport of Core PR 10775
https://github.com/bitcoin/bitcoin/pull/10775/files

Test Plan:
ninja check
test_runner.py

Reviewers: deadalnix, Fabien, #bitcoin_abc

Reviewed By: Fabien, #bitcoin_abc

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2691
…en parameter is a reorg'd block

Summary:
876e92b Testing: listsinceblock should display all transactions that were affected since the given block, including transactions that were removed due to a reorg. (Karl-Johan Alm)
f999c46 listsinceblock: optionally find and list any transactions that were undone due to reorg when requesting a non-main chain block in a new 'removed' array. (Karl-Johan Alm)

Pull request description:

  The following scenario will not notify the caller of the fact `tx0` has been dropped:

  1. User 1 receives BTC in tx0 from utxo1 in block aa1.
  2. User 2 receives BTC in tx1 from utxo1 (same) in block bb1
  3. User 1 sees 2 confirmations at block aa3.
  4. Reorg into bb chain.
  5. User 1 asks `listsinceblock aa3` and does not see that tx0 is now invalidated.

  See `listsinceblock.py` commit for related test.

  The proposed fix is to iterate from the given block down to the fork point, and to check each transaction in the blocks against the wallet, in addition to including all transactions from the fork point to the active chain tip (the current behavior). Any transactions that were present will now also be listed in the `listsinceblock` output in a new `replaced` array. This operation may be a bit heavy but the circumstances (and perceived frequency of occurrence) warrant it, I believe.

  Example output:
  ```Python
  {
    'transactions': [],
    'replaced': [
      {
        'walletconflicts': [],
        'vout': 1,
        'account': '',
        'timereceived': 1485234857,
        'time': 1485234857,
        'amount': '1.00000000',
        'bip125-replaceable': 'unknown',
        'trusted': False,
        'category': 'receive',
        'txid': 'ce673859a30dee1d2ebdb3c05f2eea7b1da54baf68f93bb8bfe37c5f09ed22ff',
        'address': 'miqEt4kWp9zSizwGGuUWLAmxEcTW9bFUnQ',
        'label': '',
        'confirmations': -7
      }
    ],
    'lastblock': '7a388f27d09e3699102a4ebf81597d974fc4c72093eeaa02adffbbf7527f6715'
  }
  ```

  I believe this addresses the comment by @luke-jr in bitcoin/bitcoin#9516 (comment) but I could be wrong..

Tree-SHA512: 607b5dcaeccb9dc0d963d3de138c40490f3e923050b29821e6bd513d26beb587bddc748fbb194503fe618cfe34a6ed65d95e8d9c5764a882b6c5f976520cff35

Backport of Core PR 9622
https://github.com/bitcoin/bitcoin/pull/9622/files
Completes T551

Test Plan:
make check
test_runner.py wallet_listsinceblock

Reviewers: deadalnix, Fabien, #bitcoin_abc

Reviewed By: Fabien, #bitcoin_abc

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2689
Summary:
8f2f1e0 wallet: Avoid second mapWallet lookup (João Barbosa)

Pull request description:

  All calls to `mapWallet.count()` have the intent to detect if a `txid` exists and most are followed by a second lookup to retrieve the `CWalletTx`.

  This PR replaces all `mapWallet.count()` calls with `mapWallet.find()` to avoid the second lookup.

Tree-SHA512: 96b7de7f5520ebf789a1aec1949a4e9c74e13683869cee012f717e5be8e51097d068e2347a36e89097c9a89f1ed1a1529db71760dac9b572e36a3e9ac1155f29

Backport of Core PR 11039
https://github.com/bitcoin/bitcoin/pull/11039/files
Completes T550
Depends on D2689

Test Plan:
ninja check
test_runner.py

Reviewers: deadalnix, Fabien, #bitcoin_abc

Reviewed By: Fabien, #bitcoin_abc

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2690
Summary:
6558f8acc [gui] Defer coin control instancing (João Barbosa)

Pull request description:

  Defer the GUI coin control instancing so that argument processing
  is taken into account for the default coin control values.

  Fixes #12312

Tree-SHA512: ecda28b94f4709319e9484b01afe763c7c3569097d2afb89db79da8a195c46d20ea77166df7edce0c8ab77627b295def01c072148714503436d27675d5e75d99

Backport of Core PR 12327
https://github.com/bitcoin/bitcoin/pull/12327/files
Completes T556

Test Plan:
make check
test_runner.py

Reviewers: deadalnix, Fabien, #bitcoin_abc

Reviewed By: Fabien, #bitcoin_abc

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2693
Summary:
* SetAddressBook(...) is locking cs_wallet internally
* DelAddressBook(...) is locking cs_wallet internally

Backport of PR11733
bitcoin/bitcoin#11733

Test Plan:
make check
test_runner.py

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

Reviewed By: jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2700
Summary:
Improve the regex:
  - Use Perl type regex in grep to remove duplication. Previously there
was 2 alsmost identical regex, one posix compliant for egrep and one
Perl compliant for python re.
  - Exclude the test folder from the source path rather than subtracting
its matching output

This improvements make the `-h` now detectable; it is added to the
undocumented exceptions.

Inspired from core PR12820.
Note that GNU `grep` is used instead of `git grep` to allow for
multiline regex (required due to our code format).

Test Plan:
Should return no error:
  arc lint

  ./test/lint/check-doc.py
There should be 1 more used argument than before this patch (`-h`),
otherwise the output should be identical.

Reviewers: #bitcoin_abc, deadalnix, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2694
Summary:
BSD grep on OSX and FreeBSD does not provide the same options as GNU
grep which prevent using them for multiline search.
This diff uses only python regex to avoid this incompatibility.
As a bonus, it runs about 3x faster.

Depends on D2694

Test Plan:
Should return no error:
  arc lint

In `src/init.cpp` delete the line 1279:
```
logger.m_print_to_console = gArgs.GetBoolArg("-printtoconsole", false);
```
Then run `arc lint` and check the linter outputs an Unknown argument
error:
  '-printtoconsole' is documented but not used'

Restore the previously deleted line in `src/init.cpp` and now delete the
lines 792 to 794:
```
strUsage += HelpMessageOpt(
        "-printtoconsole",
        _("Send trace/debug info to console instead of debug.log
file"));
```
Then run `arc lint` and check the linter outputs an Undocumented
argument error:
  '-printtoconsole' is undocumented'

Reviewers: #bitcoin_abc, deadalnix, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2695
Summary:
Use of `sprintf` is seen as a red flag as many of its uses are insecure.
OpenBSD warns about it while compiling, and some modern platforms, e.g.
[cloudlibc from cloudabi](https://github.com/NuxiNL/cloudlibc) don't
even provide it anymore.

Backport of core PR9867

Test Plan:
  make check

Reviewers: #bitcoin_abc, deadalnix, jasonbcox

Reviewed By: #bitcoin_abc, deadalnix, jasonbcox

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2697
Summary:
659b206 Make listsinceblock refuse unknown block hash (Russell Yanofsky)

Pull request description:

  Change suggested by @theuni  who noticed listsinceblock would ignore invalid block hashes causing it to return a completely unfiltered list of transactions.

Tree-SHA512: 3c8fb160265780d1334e856e853ab48e2e18372b8f1fc71ae480c3f45317048cc1fee0055d5c58031981a91b9c2bdbeb8e49a889d04ecba61729ce8109f2ce3f

Backport of Core PR 11565
https://github.com/bitcoin/bitcoin/pull/11565/files
Completes T563

Test Plan:
ninja check
wallet_listsinceblock.py

Reviewers: deadalnix, Fabien, #bitcoin_abc

Reviewed By: Fabien, #bitcoin_abc

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2703
Summary:
6bbdafc Pass serialization flags and whether to include hex to TxToUniv (Andrew Chow)
e029c6e Only return hex field once in getrawtransaction (Andrew Chow)

Pull request description:

  The hex is already returned in `TxToUniv()`, no need to give it out a second time in getrawtransaction itself.

Tree-SHA512: 270289f2d6dea37f51f5a42db3dae5debdbe83c6b504fccfd3391588da986ed474592c6655d522dc51022d4b08fa90ed1ebb249afe036309f95adfe3652cb262

Backport of Core PR 11027
bitcoin/bitcoin#11027
Completes T564

Test Plan:
ninja check
test_runner.py

Run `bitcoin-cli <txid> 1` and ensure that there is a single top-level `"hex"` field in the returned JSON.

Reviewers: deadalnix, Fabien, #bitcoin_abc

Reviewed By: Fabien, #bitcoin_abc

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2709
Summary:
Partial backport of Core PR 10742 (RetFormat only)
https://github.com/bitcoin/bitcoin/pull/10742/files
Progess towards T549

Test Plan: ninja check

Reviewers: deadalnix, Fabien, #bitcoin_abc

Reviewed By: Fabien, #bitcoin_abc

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2712
Summary:
Partial backport of Core PR 10742 (BlockSource only)
https://github.com/bitcoin/bitcoin/pull/10742/files
Progress towards T549

Test Plan: ninja check

Reviewers: deadalnix, Fabien, #bitcoin_abc

Reviewed By: Fabien, #bitcoin_abc

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2713
Summary:
Partial backport of Core PR 10742 (HelpMessageMode only)
https://github.com/bitcoin/bitcoin/pull/10742/files
Progress towards T549

Test Plan: ninja check

Reviewers: deadalnix, Fabien, #bitcoin_abc

Reviewed By: Fabien, #bitcoin_abc

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2714
Summary:
Partial backport of Core PR 10742 (FlushStateMode only)
https://github.com/bitcoin/bitcoin/pull/10742/files
Progress towards T549

Test Plan: ninja check

Reviewers: deadalnix, Fabien, #bitcoin_abc

Reviewed By: Fabien, #bitcoin_abc

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2716
Summary:
Partial backport of Core PR 10742 (VerifyResult only)
https://github.com/bitcoin/bitcoin/pull/10742/files
Progress towards T549

Test Plan: ninja check

Reviewers: deadalnix, Fabien, #bitcoin_abc

Reviewed By: Fabien, #bitcoin_abc

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2718
Summary: As per title

Test Plan:
  mkdir -p src/secp256k1/build
  cd src/secp256k1
  mkdir -p src/java/guava
  wget https://search.maven.org/remotecontent?filepath=com/google/guava/guava/18.0/guava-18.0.jar -O src/java/guava/guava-18.0.jar
  ./autogen.sh
  cd build
  ../configure --enable-jni --enable-experimental --enable-module-ecdh
  make check-java
(`make check-java` may pass or fail depending of D2685)
Use `make clean` to remove the .class files generated by `javac`.

Reviewers: #bitcoin_abc, deadalnix, jasonbcox

Reviewed By: #bitcoin_abc, jasonbcox

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2717
Summary:
This file is no longer used in the seeder since it is included in the
main tree (src/compat.h is used).

Test Plan:
  ./autogen.sh
  mkdir build && cd build
  ../configure
  make
  ./src/bitcoin-seeder
Ensure the software runs as expected

  cd ..
  mkdir buildcmake && cd buildcmake
  cmake -GNinja ..
  ninja
  ./src/bitcoin-seeder
Ensure the software runs as expected

Reviewers: #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2728
Summary: fix java secp256k1 test

Test Plan:
to run the java secp256k1 tests (including this one):

```
$ cd src/secp256k1
$ ./autogen.sh
$ ./configure --enable-jni --enable-experimental --enable-module-ecdh
$ make check-java
```

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

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, Fabien, jasonbcox

Subscribers: jasonbcox, teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2685
Summary:
4d51e9b Assert ConnectBlock block and pIndex are the same block (NicolasDorier)
972714c pow: GetNextWorkRequired never called with NULL pindexLast (Daniel Cousens)
cc44c8f ContextualCheckBlockHeader should never have pindexPrev to NULL (NicolasDorier)

Tree-SHA512: 7cc568bf9417267c335f21ec3d1505b26e56e5b3d5f4d3dbb555279489800aaa65a3bcd7bc376e274dd102912aec16ddbb18de2e2060b2667b41eb979cd9321e

Backport of Core PR 8665
https://github.com/bitcoin/bitcoin/pull/8665/files

Test Plan:
make check
test_runner.py

Reviewers: deadalnix, Fabien, #bitcoin_abc

Reviewed By: Fabien, #bitcoin_abc

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2722
…oints

Summary:
85c82b5 Avoid masking of difficulty adjustment errors by checkpoints (Pieter Wuille)

Pull request description:

  Currently difficulty adjustment violations are not reported for chains that branch off before the last checkpoint. Change this by moving the checkpoint check after the difficulty check.

Tree-SHA512: 33666f2c3459151b28c42041a463779e6df18f61d3dd5b1879a0af4e5b199ef74d1e33e06af68bebfdfb211569ad5fb56556bfebe9d63b5688d910ea211b839a

Backport of Core PR 11028
https://github.com/bitcoin/bitcoin/pull/11028/files
Depends on D2722

Test Plan:
```
make check
test_runner.py
```

Reviewers: deadalnix, Fabien, #bitcoin_abc

Reviewed By: Fabien, #bitcoin_abc

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2723
Summary: remove overkill depencency to guava

Test Plan:
execute previous tests successfully

the JNI part can be built and tested by simply doing

```
$ ./autogen.sh
$ ./configure --enable-jni --enable-experimental --enable-module-ecdh
$ make check-java
```

Once https://reviews.bitcoinabc.org/D2686 is passed we can do this automatically

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

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix, Fabien

Subscribers: Fabien, teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2681
Summary:
Backport of PR11529: bitcoin/bitcoin#11529
Completes T565

Test Plan:
make check
test_runner.py

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

Reviewed By: Fabien, O1 Bitcoin ABC, #bitcoin_abc

Subscribers: teamcity, schancel

Differential Revision: https://reviews.bitcoinabc.org/D2710
Summary: remove unused bytearray in verify

Test Plan: re run tests

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix

Subscribers: schancel

Differential Revision: https://reviews.bitcoinabc.org/D2734
@jonspock jonspock merged commit 251d805 into master Apr 8, 2019
@ghost ghost removed the in progress label Apr 8, 2019
@jonspock jonspock deleted the abc_19.2 branch April 8, 2019 20:45
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.

8 participants