stacks-network / stacks-core

The Stacks blockchain implementation
https://docs.stacks.co
GNU General Public License v3.0
3.01k stars 671 forks source link

Fix/5205 #5206

Closed jcnelson closed 1 month ago

jcnelson commented 2 months ago

Fixes #5205 so that Nakamoto network state machines run at most once per call to PeerNetwork::run()

jcnelson commented 2 months ago

This LGTM, but is it feasible to add a test for this behavior?

Arguably, the entire unit / integration test battery is the test. If this code failed to run the requisite state machines correctly, we'd see widespread failures.

jcnelson commented 1 month ago

nakamoto_integrations::clarity_burn_state is failing, which is very unexpected

jcnelson commented 1 month ago

nakamoto_integrations::clarity_burn_state passed locally for me. Probably a timing issue that got introduced by speeding up the p2p main loop.

jferrant commented 1 month ago

Awesome. Thanks for fixing that @jferrant!

No worries at all :D Thanks for explaining to me!

wileyj commented 1 month ago

results of the CI failing test (fyi, the cache has been evicted so a full re-run would be required here).

tests::nakamoto_integrations::multiple_miners)

passes

tests::signer::v0::partial_tenure_fork

RUST_BACKTRACE=full BITCOIND_TEST=1 cargo nextest run \
  --no-capture \
  --verbose \
  --tests \
  --workspace \
  --bin stacks-node \
  --run-ignored all \
  --retries 0  \
  --no-fail-fast \
  -E "test(=tests::signer::v0::partial_tenure_fork)"
...
INFO [1726789407.915984] [stackslib/src/net/p2p.rs:4840] [relayer-http://127.0.0.1:51026] Transaction rejected from mempool, {"error":"transaction rejected","reason":"TooMuchChaining","reason_data":{"actual":34,"expected":27,"is_origin":true,"message":"Nonce would exceed chaining limit in mempool","principal":"ST158X7SZWJVG8KET06C64FSRXD0V1ZT31T72SAR5"},"txid":"2aed82542ae19f3509082950d87990e04dd16838465bc0579da97fd94c968832"}
INFO [1726789407.946710] [stackslib/src/net/rpc.rs:556] [p2p-(127.0.0.1:51023,127.0.0.1:51024)] Handled StacksHTTPRequest, verb: POST, path: /v2/mempool/query?page_id=f2cf05a7cb5725983f36cbfa24dbc82566698aa6dacbdf8b658b706c6c4c28fd, processing_time_ms: 0, latency_ms: 0, conn_id: 685, peer_addr: 127.0.0.1:59792, p2p_msg: None
ERRO [1726789410.844647] [testnet/stacks-node/src/tests/nakamoto_integrations.rs:638] [tests::signer::v0::partial_tenure_fork] Timed out waiting for check to process
ERRO [1726789410.844719] [testnet/stacks-node/src/tests/signer/v0.rs:3819] [tests::signer::v0::partial_tenure_fork] Next tenure failed to tick, fork_initiated?: true, miner_1_tenures: 2, miner_2_tenures: 6, min_miner_1_tenures: 1, min_miner_2_tenures: 1, proposed_before_1: 19, proposed_before_2: 35, mined_before_1: 19, mined_before_2: 0, mined_1: 19, mined_2: 0, proposed_1: 19, proposed_2: 35
thread 'tests::signer::v0::partial_tenure_fork' panicked at testnet/stacks-node/src/tests/signer/v0.rs:3835:25:
explicit panic

tests::signer::v0::retry_on_rejection

ran for over 10 minutes, same scrolling WARN

RUST_BACKTRACE=full BITCOIND_TEST=1 cargo nextest run \
  --no-capture \
  --verbose \
  --tests \
  --workspace \
  --bin stacks-node \
  --run-ignored all \
  --retries 0  \
  --no-fail-fast \
  -E "test(=tests::signer::v0::retry_on_rejection)"
...
WARN [1726789900.947496] [stacks-signer/src/v0/signer.rs:201] [signer_runloop:3002] Cycle #11 Signer #1: Failed to push block to stacks node: Backoff retry timeout occurred. Stacks node may be down.. Retrying...

tests::signer::v0::empty_sortition

RUST_BACKTRACE=full BITCOIND_TEST=1 cargo nextest run \
  --no-capture \
  --verbose \
  --tests \
  --workspace \
  --bin stacks-node \
  --run-ignored all \
  --retries 0  \
  --no-fail-fast \
  -E "test(=tests::signer::v0::empty_sortition)"
  ...
INFO [1726790353.644569] [stackslib/src/net/rpc.rs:556] [p2p-(127.0.0.1:46863,127.0.0.1:65534)] Handled StacksHTTPRequest, verb: GET, path: /v2/info, processing_time_ms: 0, latency_ms: 0, conn_id: 645, peer_addr: 127.0.0.1:38172, p2p_msg: None
ERRO [1726790353.645066] [testnet/stacks-node/src/tests/nakamoto_integrations.rs:623] [tests::signer::v0::empty_sortition] Timed out waiting for block to process, trying to continue test
thread 'tests::signer::v0::empty_sortition' panicked at testnet/stacks-node/src/tests/signer/v0.rs:282:10:
called `Result::unwrap()` on an `Err` value: "Timed out"
stack backtrace:
obycode commented 1 month ago

partial_tenure_fork flakiness is fixed in #5197.

blockstack-devops commented 3 weeks ago

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.