Skip to content

Conversation

@theStack
Copy link
Contributor

This PR adds missing test coverage for the bumpfee RPC error "Transaction has descendants in the mempool",

if (wallet.chain().hasDescendantsInMempool(wtx.GetHash())) {
errors.push_back(Untranslated("Transaction has descendants in the mempool"));
return feebumper::Result::INVALID_PARAMETER;
}

which is thrown if the bumped tx has descendants in the mempool and is not connected to the bitcoin wallet (for those, the error "Transaction has descendants in the Wallet" is thrown a few lines above). To achieve that, the test framework's MiniWallet is used.

This commit adds missing test coverage for the bumpfee RPC
error "Transaction has descendants in the mempool", which is
thrown if the bumped tx has descendants in the mempool and is
_not_ connected to the bitcoin wallet. To achieve that, the
test framework's MiniWallet is used.
@fanquake fanquake added the Tests label Oct 15, 2021
Copy link
Contributor

@brunoerg brunoerg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tACK 4ac8c89

Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review ACK 4ac8c89. Nice stuff!

It looks like these are still untested:

  • "Transaction has been mined, or is conflicted with a mined transaction"
  • "Cannot bump transaction %s which was already bumped by transaction %s".

Copy link
Contributor

@lsilva01 lsilva01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested ACK 4ac8c89 cad756b10c9dee2d9e1405 on Ubuntu 20.04.

Copy link
Contributor

@stratospher stratospher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested ACK 4ac8c89.

@maflcko maflcko merged commit 548ad5e into bitcoin:master Oct 21, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Oct 21, 2021
…descendants in mempool

4ac8c89 test: check that bumpfee RPC fails for txs with descendants in mempool (Sebastian Falbesoner)

Pull request description:

  This PR adds missing test coverage for the bumpfee RPC error _"Transaction has descendants in the mempool"_,

  https://github.com/bitcoin/bitcoin/blob/6419bdfeb130b20ccfed229d9ba7eca7f385d036/src/wallet/feebumper.cpp#L29-L32

  which is thrown if the bumped tx has descendants in the mempool and is _not_ connected to the bitcoin wallet (for those, the error "Transaction has descendants in the Wallet" is thrown a few lines above). To achieve that, the test framework's MiniWallet is used.

ACKs for top commit:
  brunoerg:
    tACK 4ac8c89
  promag:
    Code review ACK 4ac8c89. Nice stuff!
  lsilva01:
    Tested ACK 4ac8c89 cad756b10c9dee2d9e1405 on Ubuntu 20.04.
  stratospher:
    tested ACK 4ac8c89.

Tree-SHA512: 83e99f9dd2b140c0c0597c0c36c9c948fa334871be40e58a5e004440698d9685661c69bb83ab937d30f692545a3799705f991b31904f2ef31a2fbc3ae1179fa8
@bitcoin bitcoin locked and limited conversation to collaborators Oct 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants