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

chore: extend the default block_proposal_timeout to 4 hours #5768

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

obycode
Copy link
Contributor

@obycode obycode commented Jan 29, 2025

Until #5729 is implemented, then there is no point in rejecting a block from a miner, no matter how late it is, since no other miner will ever try to extend into its tenure.

Fixes #5753

Until #5729 is implemented, then there is no point in rejecting a block
from a miner, no matter how late it is, since no other miner will ever
try to extend into its tenure.

Fixes #5753
@obycode obycode requested review from a team as code owners January 29, 2025 20:39
@obycode obycode added this to the 3.1.0.0.5 milestone Jan 29, 2025
@obycode obycode requested review from jferrant and kantai January 29, 2025 20:42
hstove
hstove previously approved these changes Jan 29, 2025
jferrant
jferrant previously approved these changes Jan 29, 2025
@aldur
Copy link
Contributor

aldur commented Jan 30, 2025

@obycode we are not sure whether the failures we are seeing are due to flakiness (probably) or a bug introduced by this PR (unlikely). In all cases, we are updating the branch, which will re-run the tests.

@obycode
Copy link
Contributor Author

obycode commented Jan 30, 2025

@obycode we are not sure whether the failures we are seeing are due to flakiness (probably) or a bug introduced by this PR (unlikely). In all cases, we are updating the branch, which will re-run the tests.

Thanks, I'll take a look. I agree, it seems unlikely that any test was inadvertently depending on that timeout, since it was already 10 minutes, but I'll investigate.

@obycode obycode dismissed stale reviews from jferrant and hstove via 38dbdcd January 30, 2025 18:08
@obycode obycode requested a review from a team as a code owner January 30, 2025 18:08
@obycode
Copy link
Contributor Author

obycode commented Jan 30, 2025

Ok, I pushed a commit to fix the flakiness in tests::nakamoto_integrations::skip_mining_long_tx.

tests::signer::v0::block_validation_pending_table looks like it just needs more time in the wait_for:

INFO [1738255286.453318] [testnet/stacks-node/src/nakamoto_node/stackerdb_listener.rs:327] [stackerdb_listener_232] StackerDBListener: Signature Added to block, block_signer_sighash: 3d43a9631e2465e2db0534dca2e617c021b9e96e4f52f0f3443067009b5c5a7f, signer_pubkey: 0223b39433cbdec8f307a4cbfd91204e8fa2cb32b57b977a726014412c50aea016, signer_slot_id: 1, signature: 011007d0c220268a75c5541eebbf7504e0b7e9182b97d9519410ee6e10652b719e0869b274543a72207a183fc5c9c73468b7008467e34f06bb8d1d80e9332bee91, signer_weight: 6, total_weight_signed: 30, tenure_extend_timestamp: 1738255360, server_version: stacks-signer 0.0.1 (:, debug build, linux [x86_64])
INFO [1738255286.453346] [testnet/stacks-node/src/nakamoto_node/signer_coordinator.rs:356] [miner.dcf9fdf3d3b3eed2e9c6b283559f09dd37e60370cde5b3cc0d147680c02a0dd8.1279939293] Received enough signatures, block accepted, block_signer_sighash: 3d43a9631e2465e2db0534dca2e617c021b9e96e4f52f0f3443067009b5c5a7f
thread 'tests::signer::v0::block_validation_pending_table' panicked at testnet/stacks-node/src/tests/signer/v0.rs:8024:6:
Timed out waiting for pending block validation to be submitted: "Timed out"
ERRO [1738255286.539887] [testnet/stacks-node/src/tests/nakamoto_integrations.rs:693] [tests::signer::v0::block_validation_pending_table] Timed out waiting for check to process

You can see the approval arrived just before the timeout, so just a little more time and it would've been fine. I'll bump that from 30s to 60s.

I need to take a closer look at tests::signer::v0::multiple_miners_empty_sortition.

@obycode
Copy link
Contributor Author

obycode commented Jan 30, 2025

I'm tempted to just bump all 30s timeouts to 60s.

@obycode
Copy link
Contributor Author

obycode commented Jan 30, 2025

tests::signer::v0::multiple_miners_empty_sortition does pass locally and the flakiness does not seem to be related to this change. I'll open an issue for it, but it does not need to hold up this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants