Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BDK will select coinbase inputs that are not matured yet #413

Closed
LLFourn opened this issue Jul 31, 2021 · 3 comments · Fixed by #459 or #614
Closed

BDK will select coinbase inputs that are not matured yet #413

LLFourn opened this issue Jul 31, 2021 · 3 comments · Fixed by #459 or #614
Assignees
Labels
bug Something isn't working

Comments

@LLFourn
Copy link
Contributor

LLFourn commented Jul 31, 2021

I just did generatetoaddress to a wallet address and tried to create a transaction. BDK wallet did it but RPC returned: "sendrawtransaction RPC error: {\"code\":-26,\"message\":\"bad-txns-premature-spend-of-coinbase, tried to spend coinbase at depth 38\"}"

@afilini
Copy link
Member

afilini commented Aug 3, 2021

I wonder what's the best way to fix this, should we ignore them completely even in the UTXO list/balance or just ignore them in the coin selection by default (maybe with a builder method to enable them explicitly)?

@LLFourn
Copy link
Contributor Author

LLFourn commented Aug 6, 2021

I think it should be in the UTXO list but should not be selected. .balance needs more work to account for things like unconfirmed vs confirmed balances #238 and "locked_coinbase_balance" could be added also added there.

@afilini
Copy link
Member

afilini commented Nov 11, 2021

Reopening since this hasn't been fixed in #459, it was just the first step

@afilini afilini reopened this Nov 11, 2021
@notmandatory notmandatory added the bug Something isn't working label Feb 15, 2022
@afilini afilini assigned afilini and danielabrozzoni and unassigned afilini Mar 7, 2022
danielabrozzoni added a commit to danielabrozzoni/bdk that referenced this issue May 25, 2022
danielabrozzoni added a commit to danielabrozzoni/bdk that referenced this issue May 25, 2022
danielabrozzoni added a commit to danielabrozzoni/bdk that referenced this issue May 26, 2022
danielabrozzoni added a commit to danielabrozzoni/bdk that referenced this issue May 26, 2022
danielabrozzoni added a commit to danielabrozzoni/bdk that referenced this issue Jun 3, 2022
danielabrozzoni added a commit to danielabrozzoni/bdk that referenced this issue Jun 13, 2022
danielabrozzoni added a commit to danielabrozzoni/bdk that referenced this issue Jun 28, 2022
danielabrozzoni added a commit to danielabrozzoni/bdk that referenced this issue Jun 28, 2022
wszdexdrf pushed a commit to wszdexdrf/bdk that referenced this issue Jun 29, 2022
wszdexdrf pushed a commit to wszdexdrf/bdk that referenced this issue Jun 29, 2022
danielabrozzoni added a commit to danielabrozzoni/bdk that referenced this issue Jun 30, 2022
rajarshimaitra pushed a commit to rajarshimaitra/bdk that referenced this issue Jul 2, 2022
@afilini afilini closed this as completed in e85aa24 Jul 5, 2022
afilini added a commit that referenced this issue Jul 5, 2022
e85aa24 Avoid using immature coinbase inputs (Daniela Brozzoni)
0e0d5a0 populate_test_db accepts a `coinbase` param (Daniela Brozzoni)

Pull request description:

  ### Description

  With this PR we start considering how many confirmations a coinbase has. If it's not mature yet, we don't use it for building transactions.
  Fixes #413

  ### Notes to the reviewers

  This PR is based on #611, review that one before reviewing this 😄

  007c5a7 adds a coinbase parameter to `populate_test_db`, to specify if you want the db to be populated with immature coins. This is useful for `test_spend_coinbase`, but that's probably going to be the only use case.
  I don't think it's a big deal to have a test function take an almost_always_useless parameter - it's not an exposed API, anyways. But, if you can come up with a different way of implementing `test_spend_coinbase` that doesn't require 007c5a7, even better! I looked for it for a while, but other than duplicating the whole `populate_test_db` code, which made the test way harder to comprehend, I didn't find any other way.

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### Bugfixes:

  * [ ] This pull request breaks the existing API
  * [x] I've added tests to reproduce the issue which are now passing
  * [x] I'm linking the issue being fixed by this PR

ACKs for top commit:
  afilini:
    ACK e85aa24

Tree-SHA512: 30f470c33f9ffe928500a58f821f8ce445c653766459465eb005031ac523c6f143856fc9ca68a8e1f23a485c6543a9565bd889f9557c92bf5322e81291212a5f
evanlinjin pushed a commit to evanlinjin/bdk that referenced this issue Jul 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants