Skip to content

Conversation

jonasschnelli
Copy link
Contributor

Right now, CWallet::CreateTransaction() has an assertion that ensures that a key could be retrieved from the keypool.

fundrawtransaction can run on an encrypted unlocked wallet (it actually doesn't sign). But, it can still fail because the keypool could be drained on a locked wallet.

Current master crashes in that case.

@gmaxwell
Copy link
Contributor

gmaxwell commented Dec 6, 2016

utACK.

@maflcko maflcko modified the milestones: 0.12.branch, 0.13.2 Dec 6, 2016
@maflcko
Copy link
Member

maflcko commented Dec 6, 2016

Tagged for 0.12 backport because I am able to reproduce on that branch:

bin/bitcoin-0.12.1/bin/bitcoin-qt -regtest -datadir=/tmp 
bitcoin-qt: wallet/wallet.cpp:2107: bool CWallet::CreateTransaction(const std::vector<CRecipient>&, CWalletTx&, CReserveKey&, CAmount&, int&, std::string&, const CCoinControl*, bool): Assertion `ret' failed.
Aborted (core dumped)

@maflcko
Copy link
Member

maflcko commented Dec 6, 2016

utACK c24a4f5

@paveljanik
Copy link
Contributor

ACK c24a4f5

What about new test case (failing in the master, OK here)?

@maflcko
Copy link
Member

maflcko commented Dec 7, 2016 via email

@jonasschnelli
Copy link
Contributor Author

Added a commit with a test that ensures the new behaviour. Test will fail on master.

@maflcko
Copy link
Member

maflcko commented Dec 7, 2016 via email

maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Dec 7, 2016
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Dec 7, 2016
@paveljanik
Copy link
Contributor

Thank you @jonasschnelli.

Tested ACK 1a6eacb

Fails on master with

Initializing test directory /var/folders/65/fn0h49r55k7779vg1b_h461r0000gn/T/testyal7ogu7/50231
Mining blocks...
Assertion failed: (ret), function CreateTransaction, file wallet/wallet.cpp, line 2392.
Unexpected exception caught during testing: ConnectionRefusedError(61, 'Connection refused')

OK here.

@maflcko maflcko modified the milestones: 0.13.2, 0.12.branch Dec 8, 2016
@gmaxwell
Copy link
Contributor

ACK.

@sipa
Copy link
Member

sipa commented Dec 10, 2016

utACK 1a6eacb

@sipa sipa merged commit 1a6eacb into bitcoin:master Dec 10, 2016
sipa added a commit that referenced this pull request Dec 10, 2016
…n keypool is empty

1a6eacb [QA] add fundrawtransaction test on a locked wallet with empty keypool (Jonas Schnelli)
c24a4f5 [Wallet] Bugfix: FRT: don't terminate when keypool is empty (Jonas Schnelli)
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Dec 14, 2016
maflcko pushed a commit to maflcko/bitcoin-core that referenced this pull request Dec 14, 2016
@maflcko
Copy link
Member

maflcko commented Dec 14, 2016

Backports:

UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request Mar 22, 2017
UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request Mar 22, 2017
UdjinM6 added a commit to dashpay/dash that referenced this pull request Apr 11, 2017
* Implement BIP 9 GBT changes

- BIP9DeploymentInfo struct for static deployment info
- VersionBitsDeploymentInfo: Avoid C++11ism by commenting parameter names
- getblocktemplate: Make sure to set deployments in the version if it is LOCKED_IN
- In this commit, all rules are considered required for clients to support

* qa/rpc-tests: bip9-softforks: Add tests for getblocktemplate versionbits updates

* getblocktemplate: Explicitly handle the distinction between GBT-affecting softforks vs not

* getblocktemplate: Use version/force mutation to support pre-BIP9 clients

* Don't use floating point

Github-Pull: bitcoin#8317
Rebased-From: 477777f

* Send tip change notification from invalidateblock

This change is needed to prevent sync_blocks timeouts in the mempool_reorg
test after the sync_blocks update in the upcoming commit
"[qa] Change sync_blocks to pick smarter maxheight".

This change was initially suggested by Suhas Daftuar <sdaftuar@chaincode.com>
in bitcoin#8680 (comment)

Github-Pull: bitcoin#9196
Rebased-From: 67c6326

* torcontrol: Explicitly request RSA1024 private key

When generating a new service key, explicitly request a RSA1024 one.

The bitcoin P2P protocol has no support for the longer hidden service names
that will come with ed25519 keys, until it does, we depend on the old
hidden service type so make this explicit.

See bitcoin#9214.

Github-Pull: bitcoin#9234
Rebased-From: 7d3b627

* Bugfix: FRT: don't terminate when keypool is empty

Github-Pull: bitcoin#9295
Rebased-From: c24a4f5

* add fundrawtransaction test on a locked wallet with empty keypool

Github-Pull: bitcoin#9295
Rebased-From: 1a6eacb
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request Jan 6, 2018
thokon00 pushed a commit to faircoin/faircoin that referenced this pull request Apr 17, 2018
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
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.

6 participants