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

bitcoind: Fix blockHash ZMQ test #2745

Merged

Conversation

alvasw
Copy link
Contributor

@alvasw alvasw commented Sep 3, 2024

The test mines blocks and waits until it sees the block hashes of the
mined blocks over ZMQ. Unfortunately, ZMQ publishers are stateless and
can't re-transmit lost messages [1]. Consequently, the test was wrong.
The fixed test subscribes to the publisher, starts mining blocks, and
waits until it observes a new block hash.

We send a `stop` RPC call and wait for the subprocess after receiving a
`success/ok` message. This is unnecessary because we received a `success`
reply already.
We parse stdout and wait for the "init message: Done loading" line to
detect when bitcoind is ready. After seeing this line, we stop reading
stdout causing the stdout buffer to fill up. When the stdout buffer
fills up, bitcoind waits until its parent process reads from it but this
unfortunately never happens leading to a deadlock.
The assumptions and expectations of the tests are wrong.
The test mines blocks and waits until it sees the block hashes of the
mined blocks over ZMQ. Unfortunately, ZMQ publishers are stateless and
can't re-transmit lost messages [1]. Consequently, the test was wrong.
The fixed test subscribes to the publisher, starts mining blocks, and
waits until it observes a new block hash.

[1] https://zguide.zeromq.org/docs/chapter5/#Pros-and-Cons-of-Pub-Sub
@alvasw alvasw added bug Something isn't working bitcoin-core labels Sep 3, 2024
Copy link
Contributor

@HenrikJannsen HenrikJannsen left a comment

Choose a reason for hiding this comment

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

utACK

@HenrikJannsen HenrikJannsen merged commit 42dc646 into bisq-network:main Sep 3, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bitcoin-core bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants