Skip to content

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Mar 21, 2022

Looks like there is no test for diamonds, only for chains (in mempool_packages.py)

@fanquake fanquake added the Tests label Mar 21, 2022
Copy link
Contributor

@jessebarton jessebarton left a comment

Choose a reason for hiding this comment

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

Concept ACK fa58577

Test passed, no issues

I'm still learning quite a bit, so not sure what else to check for. Looking at the prioritisetransaction function and trying to understand the logic.

If reviews like this aren't helpful let me know.

MarcoFalke added 2 commits March 24, 2022 14:33
* Add fallback for utxos_to_spend if none are provided
* Refactor a for-loop
@maflcko maflcko force-pushed the 2203-test-diamond- branch from fa470b6 to fa0758e Compare March 24, 2022 13:34
@maflcko
Copy link
Member Author

maflcko commented Mar 24, 2022

Rebased for minor rework

@jamesob
Copy link
Contributor

jamesob commented Mar 25, 2022

ACK fa0758e

Nice test! Ran locally. Was this motivated by the oversight in #24364?

@maflcko
Copy link
Member Author

maflcko commented Mar 25, 2022

This probably can't catch #24364 in the current form. I'd suspect there will need to be at least one call to getblocktemplate

@maflcko maflcko merged commit 3297f5c into bitcoin:master Mar 28, 2022
@maflcko maflcko deleted the 2203-test-diamond-👟 branch March 28, 2022 07:24
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 3, 2022
fa0758e test: Add diamond-shape prioritisetransaction test (MarcoFalke)
fa450c1 test: Rework create_self_transfer_multi (MarcoFalke)

Pull request description:

  Looks like there is no test for diamonds, only for chains (in `mempool_packages.py`)

ACKs for top commit:
  jamesob:
    ACK bitcoin@fa0758e

Tree-SHA512: d261184a81df77d24fc256f58ad5ed4a13b7cd4e33f74c8b79495c761ff417817602d8e5d4f63f4bb1000ac63f89bbfa54d8d8994a7b2bb2e8a484c467330984
@bitcoin bitcoin locked and limited conversation to collaborators Mar 28, 2023
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.

4 participants