1107 Commits

Author SHA1 Message Date
Harris
1a3a256d5e
wallet: replace raw pointer with const reference in AddrToPubKey 2019-11-24 22:53:42 +01:00
Samuel Dobson
8aac85d71e
Merge #17371: Refactor: Require scriptPubKey to get wallet SigningProvider
d0dab897afaac0a18aa47d3ce673a4a43a69178a Refactor: Require scriptPubKey to get wallet SigningProvider (Andrew Chow)
4b0c718f8f48c678cbe4575e9a9cf9e62a30f0da Accumulate result UniValue in SignTransaction (Andrew Chow)

Pull request description:

  Easier to review ignoring whitespace:

      git log -p -n1 -w

  This commit does not change behavior. It passes new CScript arguments to
  signing functions, but the arguments aren't currently used.

  Split from #17261

ACKs for top commit:
  instagibbs:
    utACK d0dab897af
  ryanofsky:
    Code review ACK d0dab897afaac0a18aa47d3ce673a4a43a69178a. Thanks for the SignTransaction update. No other changes since last review
  Sjors:
    Code review ACK d0dab897afaac0a18aa47d3ce673a4a43a69178a
  promag:
    Code review ACK d0dab897afaac0a18aa47d3ce673a4a43a69178a.
  meshcollider:
    Code review ACK d0dab897afaac0a18aa47d3ce673a4a43a69178a

Tree-SHA512: c3f52df20fd9d6b3b5aa65562cf5f7dce7b7f44c148b0f988f8b578fce2a28e9b7bf010f5f04bb5bf60f5272b2899f1dbbfb8aee81579c21c9cba559d1d2bb70
2019-11-23 08:35:10 +13:00
Wladimir J. van der Laan
a739d207a3
Merge #17519: rpc: Remove unused COINBASE_FLAGS
e9a27cf338dc618b8ecab8984abc54d588de8a05 refactor: Remove unused COINBASE_FLAGS (Neha Narula)

Pull request description:

  Commit d449772cf69c01932fc5d72c46054815d6300b3c stopped setting
  COINBASE_FLAGS, and it looks like it hasn't been used since P2SH.

  Following up on #17489, remove COINBASE_FLAGS which is unused. I verified that removing this did not change the contents of the coinbase's scriptSig.

ACKs for top commit:
  laanwj:
    ACK e9a27cf338dc618b8ecab8984abc54d588de8a05
  MarcoFalke:
    ACK e9a27cf338dc618b8ecab8984abc54d588de8a05 💻

Tree-SHA512: f9dac124ce7e3edcae974137764bb5039387b1b123b86af44486e398aa4a8d91a9ecf640e207b364ae303acbbaee7cca300d303ea3d6869ba9cae2bf555a6334
2019-11-22 09:37:01 +01:00
Neha Narula
e9a27cf338 refactor: Remove unused COINBASE_FLAGS
Commit d449772cf69c01932fc5d72c46054815d6300b3c stopped setting
COINBASE_FLAGS, and it looks like it hasn't been used since P2SH.
Update the help string to remove "flags", which is not specified in
BIP 22.
2019-11-20 19:06:52 -05:00
Andrew Chow
4b0c718f8f Accumulate result UniValue in SignTransaction
SignTransaction will be called multiple times in the future. Pass
it a result UniValue so that it can accumulate the results of multiple
SignTransaction passes.
2019-11-18 15:28:15 -05:00
MarcoFalke
fac07f2038
node: Add reference to mempool in NodeContext
Currently it is an alias to the global ::mempool and should be used as
follows.

* Node code (validation and transaction relay) can use either ::mempool
  or node.mempool, whichever seems a better fit.
* RPC code should use the added convenience getter EnsureMempool, which
  makes sure the mempool exists before use. This prepares the RPC code
  to a future where the mempool might be disabled at runtime or compile
  time.
* Test code should use m_node.mempool directly, as the mempool is always
  initialized for tests.
2019-11-15 13:40:00 -05:00
Wladimir J. van der Laan
45e65376ac
Merge #17382: rpc: Remove unused boost::this_thread::interruption_point
fa5facd3e72b6d61374b0b93b722b55e2b090020 rpc: Remove unused boost::this_thread::interruption_point (MarcoFalke)

Pull request description:

  There are predefined interruption points for `boost::thread`: https://www.boost.org/doc/libs/1_71_0/doc/html/thread/thread_management.html#interruption_points

  However, the rpc threads are `std::thread`, which does not have an `std:🧵:interrupt` member function to request interruption: https://dev.visucore.com/bitcoin/doxygen/httpserver_8cpp.html#ae1a63374e18b9abd348eb74e4243ea34

  Thus, the interruption points can be removed.

ACKs for top commit:
  laanwj:
    ACK fa5facd3e72b6d61374b0b93b722b55e2b090020, this does nothing.
  practicalswift:
    ACK fa5facd3e72b6d61374b0b93b722b55e2b090020
  jamesob:
    ACK fa5facd3e7

Tree-SHA512: 4e29a44df1f2702cbd1ffdffa559440a8bb800baab64b4116e2c3d27cd64d8d1e8aafe1dc21b1a4e3988470d03be19cae294bd5669f7abf6d487685dc8fd8d7e
2019-11-06 00:04:38 +01:00
MarcoFalke
fa5facd3e7
rpc: Remove unused boost::this_thread::interruption_point 2019-11-05 14:00:03 -05:00
Wladimir J. van der Laan
b05b28183c
Merge #16899: UTXO snapshot creation (dumptxoutset)
92b2f5306ba0b3f031293cb8f415b67cb002c2f1 test: add dumptxoutset RPC test (James O'Beirne)
c1ccbc3ddef931896a7e9dcfa6704e305a69fbff devtools: add utxo_snapshot.sh (James O'Beirne)
57cf74c9918d10c69a46e6ceb3cb1a5e04edf5bc rpc: add dumptxoutset (James O'Beirne)
92fafb3a7da66f737e960e541fcfbcadedf6043a coinstats: add coins_count (James O'Beirne)
707fde7b9ba522c22179e2db0ed7b462c65138d9 add unused SnapshotMetadata class (James O'Beirne)

Pull request description:

  This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11):

  Parent PR: #15606
  Issue: #15605
  Specification: https://github.com/jamesob/assumeutxo-docs/tree/master/proposal

  ---

  This changeset defines the serialization format for UTXO snapshots and adds an RPC command for creating them, `dumptxoutset`. It also adds a convenience script for generating and verifying snapshots at a certain height, since that requires doing a hacky rewind of the chain via `invalidateblock`.

  All of this is unused at the moment.

ACKs for top commit:
  laanwj:
    ACK 92b2f5306ba0b3f031293cb8f415b67cb002c2f1

Tree-SHA512: 200dff87767f157d627e99506ec543465d9329860a6cd49363081619c437163a640a46d008faa92b1f44fd403bfc7a7c9e851c658b5a4849efa9a34ca976bf31
2019-11-05 19:40:18 +01:00
James O'Beirne
57cf74c991 rpc: add dumptxoutset
Allows the creation of a UTXO snapshot to disk.
2019-11-05 13:35:57 -05:00
MarcoFalke
94a26b192f
Merge #17318: replace asserts in RPC code with CHECK_NONFATAL and add linter
c98bd13e675fbf5641ed64d551b63aaf55a1a8e9 replace asserts in RPC code with CHECK_NONFATAL and add linter (Adam Jonas)

Pull request description:

  - Replace instances of assert in /rpc files and rpcwallet with CHECK_NONFATAL(condition)
  - Add a linter to prevent future usage of assert being used in RPC code

  ref https://github.com/bitcoin/bitcoin/pull/17192

ACKs for top commit:
  practicalswift:
    ACK c98bd13e675fbf5641ed64d551b63aaf55a1a8e9 -- diff looks correct

Tree-SHA512: a16036b6bbcca73a5334665f66e17e1756377d582317568291da1d727fc9cf8c84bac9d9bd099534e1be315345336e5f7b66b93793135155f320dc5862a2d875
2019-11-04 11:33:41 -05:00
MarcoFalke
73b26e38d7
Merge #17351: doc: Fix some misspellings
ac831339cbfa65b1f7576c53b5d9a94841db9868 doc: Fix some misspellings (randymcmillan)

Pull request description:

  Here is a more thorough lint-spelling update.
  This PR takes care of easy to fix spelling errors to clean up the linting stages.
  There are misspellings coded into the functional tests.
  That is a whole separate job within itself.

ACKs for top commit:
  practicalswift:
    ACK ac831339cbfa65b1f7576c53b5d9a94841db9868 -- diff looks correct

Tree-SHA512: d8fad83fed083715655f148263ddeffc6752c8007d568fcf3dc2c418ccd5db70089ce3ccfd3994fcbd78043171402eb9cca5bdd5125287e22c42ea305aaa6e9d
2019-11-04 08:03:48 -05:00
Wladimir J. van der Laan
c4b8dd2060
Merge #17297: refactor: Remove addrdb.h dependency from node.h
f44abe4bed25a40145ab168adc1589f5df4146f3 refactor: Remove addrdb.h dependency from node.h (Hennadii Stepanov)

Pull request description:

  `node.h` includes `addrdb.h` just for the sake of `banmap_t` type.
  This PR makes dependencies simpler and explicit.

  ~Also needless `typedef` has been removed from `enum BanReason`.~

ACKs for top commit:
  laanwj:
    ACK f44abe4bed25a40145ab168adc1589f5df4146f3
  practicalswift:
    ACK f44abe4bed25a40145ab168adc1589f5df4146f3

Tree-SHA512: 33a1be20e5c629daf4a61ebbf93ea6494b9256887cebd4974de4782f6d324404b6cc84909533d9502b2cc19902083f1f9307d4fb7231e67db5b412b842d13072
2019-11-04 13:18:27 +01:00
randymcmillan
ac831339cb
doc: Fix some misspellings 2019-11-04 04:22:53 -05:00
Wladimir J. van der Laan
463eab5e14
Merge #17285: doc: Bip70 removal follow-up
3ed8e3d079a3860dcdf944f7c1aa37765a53da32 doc: Remove explicit network name references (Fabian Jahr)
d6e493f0c2850b522a676a005935163beddaa2cc wallet: Remove left-over BIP70 comment (Fabian Jahr)

Pull request description:

  A small follow-up to #17165 which removed BIP70 support.

  1. Removes one leftover mention of BIP70 in a comment.
  2. Removes BIP70 reference in comments on network/chain name strings. These can be removed as they are not really helpful and also incorrect: BIP70 only defines "main" and "test" but not "regtest". If/When signet gets merged we will add another name to the list that is not defined in BIP70. Mostly there is also an exhaustive list of the options included in the comment anyway.

  If we would like to keep an identifier for this naming scheme, I would suggest switching to something more generic, like 'short chain name'. Happy to implement that if that is preferred. Alternatively, we could add a reference to `CBaseChainParams`. That would also mean we don't have to change these lines again for signet.

ACKs for top commit:
  MarcoFalke:
    ACK 3ed8e3d079a3860dcdf944f7c1aa37765a53da32

Tree-SHA512: 9a7c0b9cacbb67bd31a089ffdc6f1ebc7f336493e2c8266eb697da34dce2b505a431d5639a3e4fc34f9287361343e861b55dc2662e0a1d2095cc1046db77d6ee
2019-11-02 14:47:41 +01:00
Fabian Jahr
3ed8e3d079 doc: Remove explicit network name references 2019-11-01 12:06:35 +01:00
Adam Jonas
c98bd13e67 replace asserts in RPC code with CHECK_NONFATAL and add linter 2019-10-30 12:03:07 -04:00
Wladimir J. van der Laan
3c40bc6726
Merge #15921: validation: Tidy up ValidationState interface
3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf [validation] Remove fMissingInputs from AcceptToMemoryPool() (John Newbery)
c428622a5bb1e37b2e6ab2c52791ac05d9271238 [validation] Remove unused first_invalid parameter from ProcessNewBlockHeaders() (John Newbery)
7204c6434b944f6ad51b3c895837729d3aa56eea [validation] Remove useless ret parameter from Invalid() (John Newbery)
1a37de4b3174d19a6d8691ae07e92b32fdfaef11 [validation] Remove error() calls from Invalid() calls (John Newbery)
067981e49246822421a7bcc720491427e1dba8a3 [validation] Tidy Up ValidationResult class (John Newbery)
a27a2957ed9afbe5a96caa5f0f4cbec730d27460 [validation] Add CValidationState subclasses (John Newbery)

Pull request description:

  Carries out some remaining tidy-ups remaining after PR 15141:

  - split ValidationState into TxValidationState and BlockValidationState (commit from ajtowns)
  - various minor code style tidy-ups to the ValidationState class
  - remove the useless `ret` parameter from `ValidationState::Invalid()`
  - remove the now unused `first_invalid` parameter from `ProcessNewBlockHeaders()`
  - remove the `fMissingInputs` parameter from `AcceptToMemoryPool()`, and deal with missing inputs the same way as other errors by using the `TxValidationState` object.

  Tip for reviewers (thanks ryanofsky!): The first commit ("[validation] Add CValidationState subclasses" ) is huge and can be easier to start reviewing if you revert the rote, mechanical changes:

  Substitute the commit hash of commit "[validation] Add CValidationState subclasses" for <CommitHash> in the commands below.

  ```sh
  git checkout <CommitHash>
  git grep -l ValidationState | xargs sed -i 's/BlockValidationState\|TxValidationState/CValidationState/g'
  git grep -l ValidationResult | xargs sed -i 's/BlockValidationResult\|TxValidationResult/ValidationInvalidReason/g'
  git grep -l MaybePunish | xargs sed -i 's/MaybePunishNode\(ForBlock\|ForTx\)/MaybePunishNode/g'
  git diff HEAD^
  ```

  After that it's possible to easily see the mechanical changes with:

  ```sh
  git log -p -n1 -U0 --word-diff-regex=. <CommitHash>
  ```

ACKs for top commit:
  laanwj:
    ACK 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf
  amitiuttarwar:
    code review ACK 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf. Also built & ran tests locally.
  fjahr:
    Code review ACK 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf . Only nit style change and pure virtual destructor added since my last review.
  ryanofsky:
    Code review ACK 3004d5a12d09d94bfc4dee2a8e8f2291996a4aaf. Just whitespace change and pure virtual destructor added since last review.

Tree-SHA512: 511de1fb380a18bec1944ea82b513b6192df632ee08bb16344a2df3c40811a88f3872f04df24bc93a41643c96c48f376a04551840fd804a961490d6c702c3d36
2019-10-30 15:37:34 +01:00
Wladimir J. van der Laan
cab94cc074
Merge #16943: test: Add generatetodescriptor RPC
fa144e6fde1f546a952023af715032cb6789d948 rpc: Add generatetodescriptor (MarcoFalke)

Pull request description:

  The existing `generatetoaddress` RPC can only generate to scriptPubKeys that can be represented by an address. However, raw scripts (such as `OP_TRUE`) or P2PK can not be represented by an address, which complicates testing.

ACKs for top commit:
  laanwj:
    ACK fa144e6fde1f546a952023af715032cb6789d948

Tree-SHA512: aee934ab7e33f07c81f3b4c8ec23e7b6ddf63a1f4b86051af0bd76b75d8da1f51627cc682e5c6e42582340ca576bbf8ff724bdd43f87128ccecfa91e52d30ae7
2019-10-30 15:22:53 +01:00
MarcoFalke
fa144e6fde
rpc: Add generatetodescriptor 2019-10-30 10:01:32 -04:00
Wladimir J. van der Laan
471e5f8829
Merge #16839: Replace Connman and BanMan globals with NodeContext local
362ded410b8cb1104b7ef31ff8488fec4824a7d5 Avoid using g_rpc_node global in wallet code (Russell Yanofsky)
8922d7f6b751a3e6b3b9f6fb7961c442877fb65a scripted-diff: Remove g_connman, g_banman globals (Russell Yanofsky)
e6f4f895d5e42feaf7bfa5f41e80292aaa73cd7d Pass NodeContext, ConnMan, BanMan references more places (Russell Yanofsky)
4d5448c76b71c9d91399c31b043237091be2e5e7 MOVEONLY: Move NodeContext struct to node/context.h (Russell Yanofsky)
301bd41a2e6765b185bd55f4c541f9e27aeea29d scripted-diff: Rename InitInterfaces to NodeContext (Russell Yanofsky)

Pull request description:

  This change is mainly a naming / organization change intended to simplify #10102. It:

  - Renames struct InitInterfaces to struct NodeContext and moves it from
    src/init.h to src/node/context.h. This is a cosmetic change intended to make
    the point of the struct more obvious.

  - Gets rid of BanMan and ConnMan globals making them NodeContext members
    instead. Getting rid of these globals has been talked about in past as a way
    to implement testing and simulations. Making them NodeContext members is a
    way of keeping them accessible without the globals.

  - Splits g_rpc_interfaces global into g_rpc_node and g_rpc_chain globals. This
    better separates node and wallet rpc methods. Node RPC methods should have
    access NodeContext, while wallet RPC methods should only have indirect access
    to node functionality via interfaces::Chain.

  - Adds NodeContext& references to interfaces::Chain class and the
    interfaces::MakeChain() function. This is needed to access ConnMan and BanMan
    instances without the globals.

  - Gets rid of redundant Node and Chain instances in Qt tests. This is
    needed due to the previous MakeChain change, and also makes test setup a
    little more straightforward. More cleanup could be done in the future, but it
    will require deduplication of bitcoind, bitcoin-qt, and TestingSetup init
    code.

ACKs for top commit:
  laanwj:
    ACK 362ded410b8cb1104b7ef31ff8488fec4824a7d5

Tree-SHA512: 9ae6ff1e33423291d1e52056bac95e0874538390892a6e83c4c115b3c73155a8827c0191b46eb3d14e3b3f6c23ccb08095490880fbc3188026319c71739f7db2
2019-10-30 12:35:41 +01:00
John Newbery
3004d5a12d [validation] Remove fMissingInputs from AcceptToMemoryPool()
Handle this failure in the same way as all other failures: call Invalid()
with the reasons for the failure.
2019-10-29 15:46:45 -04:00
John Newbery
c428622a5b [validation] Remove unused first_invalid parameter from ProcessNewBlockHeaders()
No callers use the returned value in first_invalid. Remove it from the
function signature and don't set it in the function.
2019-10-29 15:46:45 -04:00
John Newbery
a27a2957ed [validation] Add CValidationState subclasses
Split CValidationState into TxValidationState and BlockValidationState
to store validation results for transactions and blocks respectively.
2019-10-29 15:46:45 -04:00
Hennadii Stepanov
f44abe4bed
refactor: Remove addrdb.h dependency from node.h 2019-10-29 11:30:12 +02:00
Russell Yanofsky
362ded410b Avoid using g_rpc_node global in wallet code
Wallet code should use interfaces::Chain and not directly access to node state.

Add a g_rpc_chain replacement global for wallet code to use, and move
g_rpc_node definition to a libbitcoin_server source file so there are link
errors if wallet code tries to access it.
2019-10-28 10:30:51 -04:00
Russell Yanofsky
8922d7f6b7 scripted-diff: Remove g_connman, g_banman globals
-BEGIN VERIFY SCRIPT-
sed -i 's:#include <interfaces/chain.h>:#include <banman.h>\n#include <interfaces/chain.h>\n#include <net.h>\n#include <net_processing.h>:' src/node/context.cpp
sed -i 's/namespace interfaces {/class BanMan;\nclass CConnman;\nclass PeerLogicValidation;\n&/' src/node/context.h
sed -i 's/std::unique_ptr<interfaces::Chain> chain/std::unique_ptr<CConnman> connman;\n    std::unique_ptr<PeerLogicValidation> peer_logic;\n    std::unique_ptr<BanMan> banman;\n    &/' src/node/context.h
sed -i '/std::unique_ptr<[^>]\+> \(g_connman\|g_banman\|peerLogic\);/d' src/banman.h src/net.h src/init.cpp
sed -i 's/g_connman/m_context.connman/g' src/interfaces/node.cpp
sed -i 's/g_banman/m_context.banman/g' src/interfaces/node.cpp
sed -i 's/g_connman/m_node.connman/g' src/interfaces/chain.cpp src/test/setup_common.cpp
sed -i 's/g_banman/m_node.banman/g' src/test/setup_common.cpp
sed -i 's/g_connman/node.connman/g' src/init.cpp src/node/transaction.cpp
sed -i 's/g_banman/node.banman/g' src/init.cpp
sed -i 's/peerLogic/node.peer_logic/g' src/init.cpp
sed -i 's/g_connman/g_rpc_node->connman/g' src/rpc/mining.cpp src/rpc/net.cpp src/rpc/rawtransaction.cpp
sed -i 's/g_banman/g_rpc_node->banman/g' src/rpc/net.cpp
sed -i 's/std::shared_ptr<CWallet> wallet =/node.context()->connman = std::move(test.m_node.connman);\n    &/' src/qt/test/wallettests.cpp
-END VERIFY SCRIPT-
2019-10-28 10:30:51 -04:00
Russell Yanofsky
e6f4f895d5 Pass NodeContext, ConnMan, BanMan references more places
So g_connman and g_banman globals can be removed next commit.
2019-10-28 10:30:51 -04:00
Russell Yanofsky
301bd41a2e scripted-diff: Rename InitInterfaces to NodeContext
-BEGIN VERIFY SCRIPT-
s() { git grep -l "$1" src | xargs sed -i "s/$1/$2/g"; }

s 'struct InitInterfaces'              'struct NodeContext'
s 'InitInterfaces interfaces'          'NodeContext node'
s 'InitInterfaces& interfaces'         'NodeContext\& node'
s 'InitInterfaces m_interfaces'        'NodeContext m_context'
s 'InitInterfaces\* g_rpc_interfaces'  'NodeContext* g_rpc_node'
s 'g_rpc_interfaces = &interfaces'     'g_rpc_node = \&node'
s 'g_rpc_interfaces'                   'g_rpc_node'
s 'm_interfaces'                       'm_context'
s 'interfaces\.chain'                  'node.chain'
s '\(AppInitMain\|Shutdown\|Construct\)(interfaces)' '\1(node)'
s 'init interfaces' 'chain clients'
-END VERIFY SCRIPT-
2019-10-28 10:30:51 -04:00
Wladimir J. van der Laan
c72906dcc1 refactor: Remove redundant c_str() calls in formatting
Our formatter, tinyformat, *never* needs `c_str()` for strings.
Remove redundant `c_str()` calls for:

- `strprintf`
- `LogPrintf`
- `tfm::format`
2019-10-28 13:31:33 +01:00
Wladimir J. van der Laan
9ae468a6d5
Merge #17192: util: Add CHECK_NONFATAL and use it in src/rpc
faeb6665362e35f573ad715ade0ef2db62d71839 util: Add CHECK_NONFATAL and use it in src/rpc (MarcoFalke)

Pull request description:

  Fixes #17181

  Currently, we use `assert` in RPC code to document logic and code assumptions. However, it seems a bit extreme to abort all of Bitcoin Core on an assert failure in one of the RPC threads. I suggest to replace all `assert`s with a macro `CHECK_NONFATAL(condition)` that throws a runtime error when the condition evaluates to `false`. That runtime error will then be returned to the rpc caller and will include instructions to report the error to our issue tracker.

ACKs for top commit:
  practicalswift:
    ACK faeb6665362e35f573ad715ade0ef2db62d71839
  laanwj:
    ACK faeb6665362e35f573ad715ade0ef2db62d71839
  ryanofsky:
    Code review ACK faeb6665362e35f573ad715ade0ef2db62d71839

Tree-SHA512: 9b748715a5e0767ac11f1324a95a3a6ec672a0e0658013492219223bda83ce4b1b447fd8183bbb235f7df5ef7dddda7666ad569544b4d61cc65f232ca7a800ec
2019-10-28 12:00:36 +01:00
Wladimir J. van der Laan
b688b859db
Merge #17004: validation: Remove REJECT code from CValidationState
9075d13153ce06cd59a45644831ecc43126e1e82 [docs] Add release notes for removal of REJECT reasons (John Newbery)
04a2f326ec0f06fb4fce1c4f93500752f05dede8 [validation] Fix REJECT message comments (John Newbery)
e9d5a59e34ff2d538d8f5315efd9908bf24d0fdc [validation] Remove REJECT code from CValidationState (John Newbery)
0053e16714323c1694c834fdca74f064a1a33529 [logging] Don't log REJECT code when transaction is rejected (John Newbery)
a1a07cfe99fc8cee30ba5976dc36b47b1f6532ab [validation] Fix peer punishment for bad blocks (John Newbery)

Pull request description:

  We no longer send BIP 61 REJECT messages, so there's no need to set
  a REJECT code in the CValidationState object.

  Note that there is a minor bug fix in p2p behaviour here. Because the
  call to `MaybePunishNode()` in `PeerLogicValidation::BlockChecked()` only
  previously happened if the REJECT code was > 0 and < `REJECT_INTERNAL`,
  then there are cases were `MaybePunishNode()` can get called where it
  wasn't previously:

  - when `AcceptBlockHeader()` fails with `CACHED_INVALID`.
  - when `AcceptBlockHeader()` fails with `BLOCK_MISSING_PREV`.

  Note that `BlockChecked()` cannot fail with an 'internal' reject code. The
  only internal reject code was `REJECT_HIGHFEE`, which was only set in
  ATMP.

  This reverts a minor bug introduced in 5d08c9c579ba8cc7b684105c6a08263992b08d52.

ACKs for top commit:
  ariard:
    ACK 9075d13, changes since last reviewed are splitting them in separate commits to ease understanding and fix nits
  fjahr:
    ACK 9075d13153ce06cd59a45644831ecc43126e1e82, confirmed diff to last review was fixing nits in docs/comments.
  ryanofsky:
    Code review ACK 9075d13153ce06cd59a45644831ecc43126e1e82. Only changes since last review are splitting the main commit and updating comments

Tree-SHA512: 58e8a1a4d4e6f156da5d29fb6ad6a62fc9c594bbfc6432b3252e962d0e9e10149bf3035185dc5320c46c09f3e49662bc2973ec759679c0f3412232087cb8a3a7
2019-10-24 10:49:45 +02:00
MarcoFalke
faeb666536
util: Add CHECK_NONFATAL and use it in src/rpc 2019-10-18 17:19:36 -04:00
Wladimir J. van der Laan
048e456fc4
Merge #17119: doc: Fix broken bitcoin-cli examples
85016e52f6adc01735beefe5a8aefcc4f0c4aa25 [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 85016e52f6adc01735beefe5a8aefcc4f0c4aa25

Tree-SHA512: b561f68f7a188dc91dab1ceb98da3ac3e232143ab2b906c90f95c6b74b584599d0f3b51f067cdd3b1153931f95b3dc385e453b1a0dde86f9cb549b94560f219d
2019-10-17 15:40:26 +02:00
practicalswift
084e17cebd Remove unused includes 2019-10-15 22:56:43 +00:00
Andrew Toth
85016e52f6 [rpc] Fix broken bitcoin-cli examples 2019-10-14 13:09:52 -04:00
John Newbery
0053e16714 [logging] Don't log REJECT code when transaction is rejected
Remove the BIP61 REJECT code from error messages and logs when a
transaction is rejected.

BIP61 support was removed from Bitcoin Core in
fa25f43ac5692082dba3f90456c501eb08f1b75c. The REJECT codes will be
removed from the codebase entirely in the following commit.
2019-10-10 11:19:42 -04:00
Wladimir J. van der Laan
ccaef6c28b
Merge #16908: txmempool: Make entry time type-safe (std::chrono)
faec689bed7a5b66e2a7675853d10205b933cec8 txmempool: Make entry time type-safe (std::chrono) (MarcoFalke)
faaa1f01daba94b021ca77515266a16d27f0364e util: Add count_seconds time helper (MarcoFalke)
1111170f2f0141084b5b4ed565b2f07eba48599a test: mempool entry time is persisted (MarcoFalke)

Pull request description:

  This changes the type of the entry time of txs into the mempool from `int64_t` to `std::chrono::seconds`.

  The benefits:
  * Documents the type for developers
  * Type violations result in compile errors
  * After compilation, the two are equivalent (at no run time cost)

ACKs for top commit:
  ajtowns:
    utACK faec689bed7a5b66e2a7675853d10205b933cec8
  laanwj:
    ACK faec689bed7a5b66e2a7675853d10205b933cec8

Tree-SHA512: d958e058755d1a1d54cef536a8b30a11cc502b7df0d6ecf84a0ab1d38bc8105a67668a99cd5087a444f6de2421238111c5fca133cdf8e2e2273cb12cb6957845
2019-10-02 16:55:36 +02:00
Wladimir J. van der Laan
a6c8aed1f1
Merge #16817: rpc: Fix casing in getblockchaininfo to be inline with other fields
1a02edb3f2803b6f82f06a31acf0b0e5fc19bd1c [RPC] Fix casing in getblockchaininfo to be inline with the rest of the response (Dan Gershony)

Pull request description:

  The response in the RPC result `startTime` is camel cased while the rest of the response seems to be lower cased.

  If this was intentional please ignore and close this PR.

  Note: RPC field case changes might break existing callers

ACKs for top commit:
  laanwj:
    ACK 1a02edb3f2803b6f82f06a31acf0b0e5fc19bd1c

Tree-SHA512: 6f0eaf2b4aaf73c9a9bf1fbd4af59af5f95fc012fa88f94e050e6ae273b3ad647f5729df53bfce91e1a925fe4fd7b14818908bb6131a81413a555137d1007d7c
2019-09-27 15:11:00 +02:00
Dan Gershony
1a02edb3f2 [RPC] Fix casing in getblockchaininfo to be inline with the rest of the response
The response in the RPC result `starttime` is camel cased while the rest of the response seems to be lower cased.

If this was intentional please ignore this PR.

Note: case might break existing callers

Reflect the change in the test data

Change to snake case
2019-09-26 15:20:55 +01:00
MarcoFalke
faec689bed
txmempool: Make entry time type-safe (std::chrono) 2019-09-23 08:00:14 -04:00
Jon Atack
5c1cd78b7e
doc: improve rawtransaction code/test docs 2019-09-21 16:01:20 +02:00
fanquake
630ec7bf41
Merge #16900: doc: Fix doxygen comment for SignTransaction in rpc/rawtransaction_util
fa8d65f07187590ae507c65a6dd63fd47b8d1fb3 doc: Fix doxygen comment for SignTransaction in rpc/rawtransaction_util (MarcoFalke)

Pull request description:

  The param `coins` to `SignTransaction` is final and can thus not be extended (as suggested by the doc).

ACKs for top commit:
  practicalswift:
    ACK fa8d65f07187590ae507c65a6dd63fd47b8d1fb3 -- const correctness is good and diff looks correct
  fanquake:
    ACK fa8d65f07187590ae507c65a6dd63fd47b8d1fb3

Tree-SHA512: 041e159f2c3cf96e296173c31f3e5f35bbc7711cc888aa4bf08aaa8c65c95ee7f7672f65396690a9af45795a618eea0fadde7fb02d29ec85f1b4df5e6d9e0c7a
2019-09-20 16:25:59 +08:00
MarcoFalke
fa8d65f071
doc: Fix doxygen comment for SignTransaction in rpc/rawtransaction_util 2019-09-19 13:34:50 -04:00
Wladimir J. van der Laan
0ee0474234
Merge #16521: rpc: Use the default maxfeerate value as BTC/kB
2dfd6834ef8737e16e4b96df0c459f30a0721d6c test: Add test for default maxfeerate in sendrawtransaction (Joonmo Yang)
261843e4bef96ab296a9775819a99bfa60cad743 wallet/rpc: Use the default maxfeerate value as BTC/kB (Joonmo Yang)

Pull request description:

  Fixes https://github.com/bitcoin/bitcoin/issues/16382

  This patch tries to treat `maxfeerate` in sendrawtransaction/testmempoolaccept RPC as a rate(BTC/kB) instead of an absolute value(BTC).
  The included test case checks if the new behavior works correctly, by using the transaction with an absolute fee of ~0.02BTC, where the fee rate is ~0.2BTC/kB.
  This test should be failing if the default `maxfeerate` is 0.1BTC, but pass if the default value is 0.1BTC/kB

ACKs for top commit:
  laanwj:
    ACK 2dfd6834ef8737e16e4b96df0c459f30a0721d6c (ACKs by Sjors and MarcoFalke above for trivially different code)

Tree-SHA512: a1795bffe8a182acef8844797955db1f60bb0c0ded97148f3572dc265234d5219271a3a7aa0b6418a43f73b2b2720ef7412ba169c99bb1cdcac52051f537d6af
2019-09-18 16:49:18 +02:00
Wladimir J. van der Laan
72d30d668a
Merge #16512: rpc: Shuffle inputs and outputs after joining psbts
c0b5d9710322a614a50ab5da081558cf6a38ad2a Test that joinpsbts randomly shuffles the inputs (Andrew Chow)
6f405a1d3b38395e35571b68aae55cae50e0762a Shuffle inputs and outputs after joining psbts (Andrew Chow)

Pull request description:

  `joinpsbts` currently just adds the inputs and outputs in the order of that the PSBTs were provided. This makes it extremely easy to identify which outputs belong to which inputs. This PR changes that so that all of the inputs and outputs are shuffled in the joined transaction.

ACKs for top commit:
  instagibbs:
    utACK c0b5d97103
  jonatack:
    ACK c0b5d9710322a614a50ab5da081558cf6a38ad2a modulo suggestions for later.

Tree-SHA512: 14a0b7aae07d92e6d2c76a3a3b228b481e1964cb7d34f97515bdda18e2ea05a9f97c5a22affc143b86ae8b95c3cb239849fb54219d65512bc2112264dca915c8
2019-09-18 16:19:47 +02:00
John Newbery
7dee8f4808 [wallet] Rename 'decode' argument in gettransaction method to 'verbose'
This makes the RPC method consistent with other RPC methods that have a
'verbose' option.

Change the name of the return object from 'decoded' to details.

Update help text.
2019-09-13 22:33:46 +03:00
fanquake
2296fe65f5
Merge #16251: Improve signrawtransaction error reporting
ec4c79326bb670c2cc1757ecfb1900f8460c5257 signrawtransaction*: improve error for partial signing (Anthony Towns)
3c481f8921bbc587cf287329f39243abe703b868 signrawtransactionwithkey: better error messages for bad redeemScript/witnessScript (Anthony Towns)

Pull request description:

  Two fixes for `signrawtransactionwith{key,wallet}` (in addition to #16250): one that checks redeemScript/witnessScript matches scriptPubKey (and if both are provided that they match each other sanely), and the other changes the warning when some-but-not-all the signatures for a CHECKMULTISIG are provided to something that suggests more signatures may be all that's required.

  Fixes: #13218
  Fixes: #14823

ACKs for top commit:
  instagibbs:
    utACK ec4c79326b
  achow101:
    Code Review ACK ec4c79326bb670c2cc1757ecfb1900f8460c5257
  meshcollider:
    utACK ec4c79326bb670c2cc1757ecfb1900f8460c5257

Tree-SHA512: 0c95c91d498e85b834662b9e5c83f336ed5fd306be7701ce1dbfa0836fbeb448a267a796585512f7496e820be668b07c2a0a2f45e52dc23f09ee7d9c87e42b35
2019-09-11 15:37:13 +10:00
Wladimir J. van der Laan
33c466a642
Merge #16787: rpc: Human readable network services
66740f460af5f9d8c61eb5b154863bffb20d94b5 doc: add a release note for the new field in 'getpeerinfo' and 'getnetworkinfo' (darosior)
6564f58c87a84c88d07629e9d86e15f07d0ed837 rpc/net: decode the services flags in a new entry (darosior)

Pull request description:

  This is a reopen of https://github.com/bitcoin/bitcoin/pull/15511#issuecomment-527087370 since there have been concept ACKs from sdaftuar and Sjors.

  This adds a new entry to `getpeerinfo` and `getnetworkinfo` which decodes the network services flags.

  Here is a truncated output of `getpeerinfo`:
  ```
  "services": "000000000000040d",
  "servicesnames": "NODE_NETWORK | NODE_BLOOM | NODE_WITNESS | NODE_NETWORK_LIMITED",
  "relaytxes": true,
  ```
  And one of `getnetworkinfo`:
  ```
  "localservices": "0000000000000409",
  "localservicesnames": "NODE_NETWORK | NODE_WITNESS | NODE_NETWORK_LIMITED",
  "localrelay": true,
  ```

  Fixes #16780.

ACKs for top commit:
  MarcoFalke:
    unsigned ACK 66740f460af5f9d8c61eb5b154863bffb20d94b5
  laanwj:
    ACK 66740f460af5f9d8c61eb5b154863bffb20d94b5

Tree-SHA512: 0acc37134b283f56004a41243903d7790cb01591ddf0342489bd05f3a2c780563075373ba5fd55180fa15632e8968ffa11a979b8afece75a6a2e891342601440
2019-09-10 09:03:14 +02:00
Anthony Towns
ec4c79326b signrawtransaction*: improve error for partial signing
Thanks to Danial Jaffy (tipu) for reporting this issue.
2019-09-10 15:41:50 +10:00