Skip to content

[pull] master from bitcoin:master#17

Merged
pull[bot] merged 18 commits intorezdh:masterfrom
bitcoin:master
Jan 25, 2022
Merged

[pull] master from bitcoin:master#17
pull[bot] merged 18 commits intorezdh:masterfrom
bitcoin:master

Conversation

@pull
Copy link
Copy Markdown

@pull pull bot commented Jan 25, 2022

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

Sjors and others added 18 commits December 9, 2021 15:49
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
In order to allow coin selection to take weights from the user,
CCoinControl needs to be able to set and get them.
If the user specifies an input size, allow it to override any input size
calculations during coin selection.
Coin selection requires knowing the weight of a transaction so that fees
can be estimated. However for external inputs, the weight may not be
avialble, and solving data may not be enough as the input could be one
that we do not support. By allowing users to specify input weights,
those external inputs can be included in the transaction.

Additionally, if the weight for an input is specified, that value will
always be used, regardless of whether the input is in the wallet or
solving data is available. This allows us to account for scenarios where
the wallet may be more conservative and estimate a larger input than may
actually be created.

For example, we assume the maximum DER signature size, but an external
input may be signed by a wallet which does nonce grinding in order to get
a smaller signature. In that case, the user can specify the smaller
input weight to avoid overpaying transaction fees.
Added tests to rpc_fundrawtransaction, wallet_send, and rpc_psbt that
test that external inputs can be spent when input weight is provided.
Also tested that the input weight overrides any calculated weight.

Additionally, rpc_psbt's external inputs test is cleaned up a bit to be
more similar to rpc_fundrawtransaction's and avoid potential pitfalls
due to non-deterministic coin selection behavior.
…ng a transaction

3866272 tests: Test specifying input weights (Andrew Chow)
6fa762a rpc, wallet: Allow users to specify input weights (Andrew Chow)
808068e wallet: Allow user specified input size to override (Andrew Chow)
4060c50 wallet: add input weights to CCoinControl (Andrew Chow)

Pull request description:

  When funding a transaction with external inputs, instead of providing solving data, a user may want to just provide the maximum signed size of that input. This is particularly useful in cases where the input is nonstandard as our dummy signer is unable to handle those inputs.

  The input weight can be provided to any input regardless of whether it belongs to the wallet and the provided weight will always be used regardless of any calculated input weight. This allows the user to override the calculated input weight which may overestimate in some circumstances due to missing information (e.g. if the private key is not known, a maximum size signature will be used, but the actual signer may be doing additional work which reduces the size of the signature).

  For `send` and `walletcreatefundedpsbt`, the input weight is specified in a `weight` field in an input object. For `fundrawtransaction`,  a new `input_weights` field is added to the `options` object. This is an array of objects consisting of a txid, vout, and weight.

  Closes #23187

ACKs for top commit:
  instagibbs:
    reACK 3866272
  glozow:
    reACK 3866272 via range-diff
  t-bast:
    ACK 3866272

Tree-SHA512: 2c8b471ee537c62a51389b7c4e86b5ac1c3a223b444195042be8117b3c83e29c0619463610b950cbbd1648d3ed01ecc5bb0b3c4f39640680da9157763b9b9f9f
923312f rpc: use peer_id, block_hash for FetchBlock (Sjors Provoost)
34d5399 rpc: more detailed errors for getblockfrompeer (Sjors Provoost)
60243ca rpc: turn already downloaded into error in getblockfrompeer (Sjors Provoost)
809d66b rpc: clarify getblockfrompeer behavior when called multiple times (Sjors Provoost)
0e3d7c5 refactor: drop redundant hash argument from FetchBlock (Sjors Provoost)
8d1a3e6 rpc: allow empty JSON object result (Sjors Provoost)
bfbf91d test: fancier Python for getblockfrompeer (Sjors Provoost)

Pull request description:

  Followups from #20295.

ACKs for top commit:
  jonatack:
    ACK 923312f 📦
  fjahr:
    tested ACK 923312f

Tree-SHA512: da9eca76e302e249409c9d7f0d16cca668ed981e2ab6ca2d1743dad0d830b94b1bc5ffb9028a00764b863201945c273cc8f4409a4c9ca3817830007dffa2bc20
fac8165 Remove unused checkFinalTx (MarcoFalke)
fa272ea wallet: Avoid dropping confirmed coins (MarcoFalke)
888841e interfaces: Remove unused is_final (MarcoFalke)
dddd05e qt: Treat unconfirmed txs as unconfirmed (MarcoFalke)

Pull request description:

  The wallet has several issues:

  ## Unconfirmed txs in the GUI

  The GUI clumsily attempts to guess if unconfirmed txs are locked until a future time. This is currently based on the locktime only, not nSequence, thus wrong. Fix this by removing the clumsy code and treat all unconfirmed txs as unconfirmed. The GUI already prints whether a tx is in the mempool, in which case the user knows that the tx wasn't locked until a future time. If the tx is not in the mempool, it might be better to report the exact reject reason from the mempool instead of using incorrect heuristics.

  ## Confirmed txs in the wallet

  The wallet drops coins that it incorrectly assumes to be locked until a future time, even if they are already confirmed in the chain. This is because the wallet is using the wrong time (adjusted network time) instead of MTP, due to the `-1` default argument of `CheckFinalTx`.

  The issues are fixed in separate commits and there is even a test.

ACKs for top commit:
  achow101:
    ACK fac8165
  prayank23:
    reACK fac8165
  glozow:
    code review ACK fac8165, I understand now how this fixes both issues.

Tree-SHA512: 210afb855f4c6d903fee49eba6b1a9735d699cf0168b669eabb38178e53b3a522258b7cc669f52489c6cd3e38bf358afde12eef3ba2e2f2ffaeb06b8f652ccd0
@pull pull bot added the ⤵️ pull label Jan 25, 2022
@pull pull bot merged commit e30b6ea into rezdh:master Jan 25, 2022
pull bot pushed a commit that referenced this pull request Feb 23, 2022
fa7e147 test: Fix intermittent Tsan issue (MarcoFalke)

Pull request description:

  Fix https://cirrus-ci.com/task/5176769937408000?logs=ci#L5161

  ```
  WARNING: ThreadSanitizer: data race (pid=22965)
    Write of size 8 at 0x7f74d5e21f50 by main thread:
      #0 std::__1::ios_base::precision(long) /usr/lib/llvm-13/bin/../include/c++/v1/ios:513:18 (test_bitcoin+0x1a8366)
      #1 boost::io::ios_base_all_saver::restore() /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/io/ios_state.hpp:341:17 (test_bitcoin+0x1a8366)
      #2 boost::unit_test::unit_test_log_t::operator<<(boost::unit_test::log::begin const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_log.ipp:336:55 (test_bitcoin+0x1a8366)
      #3 boost::test_tools::tt_detail::report_assertion(boost::test_tools::assertion_result const&, boost::unit_test::lazy_ostream const&, boost::unit_test::basic_cstring<char const>, unsigned long, boost::test_tools::tt_detail::tool_level, boost::test_tools::tt_detail::check_type, unsigned long, ...) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/test_tools.ipp:359:19 (test_bitcoin+0x1b3b9b)
      #4 txindex_tests::txindex_initial_sync::test_method() src/test/txindex_tests.cpp:31:5 (test_bitcoin+0x78aebc)
      #5 txindex_tests::txindex_initial_sync_invoker() src/test/txindex_tests.cpp:16:1 (test_bitcoin+0x78a384)
      #6 boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11 (test_bitcoin+0x2bf30d)
      #7 boost::function0<void>::operator()() const /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x224027)
      #8 boost::detail::forward::operator()() /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1368:32 (test_bitcoin+0x224027)
      #9 boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:137:18 (test_bitcoin+0x224027)
      #10 boost::function0<int>::operator()() const /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x1ac66c)
      #11 int boost::detail::do_invoke<boost::shared_ptr<boost::detail::translator_holder_base>, boost::function<int ()> >(boost::shared_ptr<boost::detail::translator_holder_base> const&, boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:290:30 (test_bitcoin+0x1ac66c)
      #12 boost::execution_monitor::catch_signals(boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:879:16 (test_bitcoin+0x1ac66c)
      #13 boost::execution_monitor::execute(boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1277:16 (test_bitcoin+0x1ac980)
      #14 boost::execution_monitor::vexecute(boost::function<void ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1377:5 (test_bitcoin+0x1a7f9b)
      #15 boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned long) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_monitor.ipp:49:9 (test_bitcoin+0x1a7f9b)
      #16 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:823:44 (test_bitcoin+0x1e0d5c)
      #17 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:792:58 (test_bitcoin+0x1e14a6)
      #18 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:792:58 (test_bitcoin+0x1e14a6)
      #19 boost::unit_test::framework::run(unsigned long, bool) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:1696:29 (test_bitcoin+0x1a6bfb)
      #20 boost::unit_test::unit_test_main(boost::unit_test::test_suite* (*)(int, char**), int, char**) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:248:9 (test_bitcoin+0x1c4ed6)
      #21 main /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:304:12 (test_bitcoin+0x1c5506)
    Previous write of size 8 at 0x7f74d5e21f50 by thread T4:
      [failed to restore the stack]
    Location is global 'std::__1::cout' of size 160 at 0x7f74d5e21f30 (libc++.so.1+0x0000000cdf50)
    Thread T4 'b-txindex' (tid=22989, running) created by main thread at:
      #0 pthread_create <null> (test_bitcoin+0x1184cd)
      #1 std::__1::__libcpp_thread_create(unsigned long*, void* (*)(void*), void*) /usr/lib/llvm-13/bin/../include/c++/v1/__threading_support:514:10 (test_bitcoin+0xa23f1b)
      #2 std::__1::thread::thread<void (*)(char const*, std::__1::function<void ()>), char const*, BaseIndex::Start(CChainState&)::$_0, void>(void (*&&)(char const*, std::__1::function<void ()>), char const*&&, BaseIndex::Start(CChainState&)::$_0&&) /usr/lib/llvm-13/bin/../include/c++/v1/thread:307:16 (test_bitcoin+0xa23f1b)
      #3 BaseIndex::Start(CChainState&) src/index/base.cpp:363:21 (test_bitcoin+0xa23f1b)
      #4 txindex_tests::txindex_initial_sync::test_method() src/test/txindex_tests.cpp:31:5 (test_bitcoin+0x78adfa)
      #5 txindex_tests::txindex_initial_sync_invoker() src/test/txindex_tests.cpp:16:1 (test_bitcoin+0x78a384)
      #6 boost::detail::function::void_function_invoker0<void (*)(), void>::invoke(boost::detail::function::function_buffer&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:117:11 (test_bitcoin+0x2bf30d)
      #7 boost::function0<void>::operator()() const /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x224027)
      #8 boost::detail::forward::operator()() /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1368:32 (test_bitcoin+0x224027)
      #9 boost::detail::function::function_obj_invoker0<boost::detail::forward, int>::invoke(boost::detail::function::function_buffer&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:137:18 (test_bitcoin+0x224027)
      #10 boost::function0<int>::operator()() const /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/function/function_template.hpp:763:14 (test_bitcoin+0x1ac66c)
      #11 int boost::detail::do_invoke<boost::shared_ptr<boost::detail::translator_holder_base>, boost::function<int ()> >(boost::shared_ptr<boost::detail::translator_holder_base> const&, boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:290:30 (test_bitcoin+0x1ac66c)
      #12 boost::execution_monitor::catch_signals(boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:879:16 (test_bitcoin+0x1ac66c)
      #13 boost::execution_monitor::execute(boost::function<int ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1277:16 (test_bitcoin+0x1ac980)
      #14 boost::execution_monitor::vexecute(boost::function<void ()> const&) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/execution_monitor.ipp:1377:5 (test_bitcoin+0x1a7f9b)
      #15 boost::unit_test::unit_test_monitor_t::execute_and_translate(boost::function<void ()> const&, unsigned long) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_monitor.ipp:49:9 (test_bitcoin+0x1a7f9b)
      #16 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:823:44 (test_bitcoin+0x1e0d5c)
      #17 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:792:58 (test_bitcoin+0x1e14a6)
      #18 boost::unit_test::framework::state::execute_test_tree(unsigned long, unsigned long, boost::unit_test::framework::state::random_generator_helper const*) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:792:58 (test_bitcoin+0x1e14a6)
      #19 boost::unit_test::framework::run(unsigned long, bool) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/framework.ipp:1696:29 (test_bitcoin+0x1a6bfb)
      #20 boost::unit_test::unit_test_main(boost::unit_test::test_suite* (*)(int, char**), int, char**) /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:248:9 (test_bitcoin+0x1c4ed6)
      #21 main /tmp/cirrus-ci-build/depends/x86_64-pc-linux-gnu/include/boost/test/impl/unit_test_main.ipp:304:12 (test_bitcoin+0x1c5506)
  SUMMARY: ThreadSanitizer: data race /usr/lib/llvm-13/bin/../include/c++/v1/ios:513:18 in std::__1::ios_base::precision(long)
  ==================
  Exit status: 2

ACKs for top commit:
  fanquake:
    CI ignored ACK fa7e147

Tree-SHA512: 5194e026410b96ad3c8addeecce0a55ee0271c3cfac9fa0715345b1a50d59925549cee0a3e415e5837ae6d2f214a7b622c73cfc7fdf41d5e55c24fb87fddb9d1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants