2.05 integration testing: exact costs matching in miner and follower: #2913 #2916

kantai · 2021-11-10T16:57:48Z

This PR does a few things:

Merges develop to next-costs
Adds the confirmed microblock cost and anchored block cost to new_block events
Adds a new mined_block event that relays information about mined, but not necessarily won blocks from a miner.
Adds an integration test that asserts exact cost consumption matching between the miner block assembly and the follower block validation paths ([chainstate] test that miner and follower have *exactly* the same block costs #2913)
Fixes a bug in the miner block assembly path that led miners to be overly pessimistic in their own block costs when confirming microblocks

…s, and replace all instances of doing this ad-hoc with a call to this method

…ws_and_then

Fix/2885

… signal handler report the signal received to the user-supplied callback.

…trlc crate's platform-specific deps

…ng -- this is an inevitable consequence of having multiple runloops

…ibc methods for recording that a signal has been caught.

…ds in ctrlc

Use mmap on MARF connections

…patcher

jcnelson · 2021-11-10T20:17:41Z

src/chainstate/stacks/miner.rs

+                .expect("BUG: block_cost + microblock_cost < block_cost");
+
+            // if we get here, then we need to reset the block-cost back to 0 because this begins the
+            // block defined by this miner.


lol copypasta

I specifically altered "epoch" to "block"! Is it more confusing? I'm not sure. But having two definitions of epochs is terrifying to me!

No, not a problem at all. Just happened to notice that almost-the-same comment is in append_block() ;)

jcnelson · 2021-11-10T20:22:59Z

testnet/stacks-node/src/event_dispatcher.rs

 pub const PATH_BURN_BLOCK_SUBMIT: &str = "new_burn_block";
 pub const PATH_BLOCK_PROCESSED: &str = "new_block";
 pub const PATH_ATTACHMENT_PROCESSED: &str = "attachments/new";

+#[derive(Clone, Serialize, Deserialize)]
+pub struct MinedBlockEvent {


Is this something the sidecar will use, or is this only meant for testing? Fine either way; just curious.

Probably not API operators, but it's something that could be useful down the road for miners (or running a mock miner for analysis) -- event observers could get all sorts of information about their miner this way. Right now, it's really only useful for testing, but I figured it could be made into something more generally useful at some point.

jcnelson

LGTM! Thanks @kantai

EDIT: can we make sure that this change doesn't break any of the overflow tests?

kantai · 2021-11-10T21:39:17Z

EDIT: can we make sure that this change doesn't break any of the overflow tests?

Yep -- that's the remaining item here, I need to look at that microblock integration test.

…ntegration_test to match new expected behavior

blockstack-devops · 2024-11-20T00:21:34Z

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.

jcnelson added 30 commits October 18, 2021 14:56

fix: add sqlite_open(), which sets the usual pragmas and busy handler…

49b4fdf

…s, and replace all instances of doing this ad-hoc with a call to this method

fix: use rusqlite's new Connection::pragma method instead of query_ro…

9cef3d3

…ws_and_then

fix: typo

2a35aa7

chore: unit test for sqlite_open() to verify that it sets pragmas

92c99b5

Merge branch 'develop' into fix/2885

3d47711

chore: remove set_wal_mode -- it's not used

34777d1

fix: merge artifact

3eac1fd

Merge pull request #2887 from blockstack/fix/2885

3b64620

Fix/2885

feat: vendor ctrlc package and add support for SIGBUS. Also, have the…

2a1b5bf

… signal handler report the signal received to the user-supplied callback.

feat: add nix crate (to support vendored ctrlc package) and add the c…

d202d26

…trlc crate's platform-specific deps

refactor: db_mkdirs() now only returns one argument

c55a6e4

chore: give credit where credit is due for ctrlc

fd962c3

chore: expose ctrlc vendored package

c76a7f5

feat: expose ctrlc crate, with OS-specific packages

4cef87b

feat: use mmap by default in sqlite connections (256MB)

36b2743

chore: no longer need ctrlc

0511532

refactor: use deps/ctrlc now

4ef95f9

fix: update docstrings to use deps::ctrlc

ede6334

fix: use move in docstring and use new arg

0cde87c

fix: add unix and windows deps for vendored ctrlc package to libclarity

cef19e6

fix: don't panic if we set a signal handler multiple times when testi…

9943a4b

…ng -- this is an inevitable consequence of having multiple runloops

refactor: only MARF databases will use mmap

88185e0

refactor: expose test module

c5a276f

refactor: run original ctrlc tests as a unit test

1e1d7c9

fix: don't mmap on sqlite_open()

13eb05e

fix: crash and burn on SIGBUS, and use async-safe (but rust unsafe) l…

0a46823

…ibc methods for recording that a signal has been caught.

fix: use proper libc types

34f2cbf

chore: new Cargo lockfile

7062a02

chore: remove needless docstring example

684e8c9

fix: have clarity target use nix and winapi for platform-specific nee…

1ff6075

…ds in ctrlc

jcnelson and others added 8 commits October 27, 2021 12:30

chore: add CHANGELOG entry about mmap'ed connections

8303d22

Merge pull request #2900 from blockstack/fix/2869

75f6328

Use mmap on MARF connections

chore: prepare changelog for release

5d021a9

Merge branch 'develop' into next-costs

fcc717d

chore: fix merge artifacts in unit tests

7abd0c2

test: start of testing for #2913. adds a mined_block event to the dis…

f688e8c

…patcher

fix: issue raised in #2913

9ee9101

docs: add reference for the event dispatcher changes

b1291da

kantai requested a review from jcnelson November 10, 2021 16:58

kantai self-assigned this Nov 10, 2021

kantai added the next-cost label Nov 10, 2021

kantai linked an issue Nov 10, 2021 that may be closed by this pull request

[chainstate] test that miner and follower have *exactly* the same block costs #2913

Closed

kantai requested a review from pavitthrap November 10, 2021 16:58

jcnelson reviewed Nov 10, 2021

View reviewed changes

jcnelson self-requested a review November 10, 2021 20:23

jcnelson approved these changes Nov 10, 2021

View reviewed changes

test: fix the assertion in runtime_overflow_unconfirmed_microblocks_i…

342a498

…ntegration_test to match new expected behavior

kantai requested a review from gregorycoppola November 11, 2021 17:35

ci: add new test to github workflow

e9c6d49

gregorycoppola approved these changes Nov 11, 2021

View reviewed changes

kantai merged commit 0b12ed0 into next-costs Nov 12, 2021

kantai deleted the test/exact-costs-2913 branch November 12, 2021 00:00

blockstack-devops added the locked label Nov 20, 2024

stacks-network locked as resolved and limited conversation to collaborators Nov 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

2.05 integration testing: exact costs matching in miner and follower: #2913 #2916

2.05 integration testing: exact costs matching in miner and follower: #2913 #2916

kantai commented Nov 10, 2021

jcnelson Nov 10, 2021

kantai Nov 10, 2021

jcnelson Nov 10, 2021

jcnelson Nov 10, 2021

kantai Nov 10, 2021

jcnelson left a comment •

edited

Loading

kantai commented Nov 10, 2021

blockstack-devops commented Nov 20, 2024

2.05 integration testing: exact costs matching in miner and follower: #2913 #2916

2.05 integration testing: exact costs matching in miner and follower: #2913 #2916

Conversation

kantai commented Nov 10, 2021

jcnelson Nov 10, 2021

Choose a reason for hiding this comment

kantai Nov 10, 2021

Choose a reason for hiding this comment

jcnelson Nov 10, 2021

Choose a reason for hiding this comment

jcnelson Nov 10, 2021

Choose a reason for hiding this comment

kantai Nov 10, 2021

Choose a reason for hiding this comment

jcnelson left a comment • edited Loading

Choose a reason for hiding this comment

kantai commented Nov 10, 2021

blockstack-devops commented Nov 20, 2024

jcnelson left a comment •

edited

Loading