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

feat: Mempool Analyzer #3020

Closed
wants to merge 6 commits into from
Closed

feat: Mempool Analyzer #3020

wants to merge 6 commits into from

Conversation

gregorycoppola
Copy link
Contributor

@gregorycoppola gregorycoppola commented Jan 27, 2022

Motivation

Towards #3002

New Code

This program can be used to "analyze the mempool". This can be used to compute statistics, especially:

  • how big is the mempool?
  • what percentage of mempool transactions are mineable?
  • what is the cost to clear the mempool?

More specifically, this tool can be used to mine an "unlimited block". That is, it will attempt to build a block from the transactions in the mempool, but will never run out of space in the block.

Each transaction in the mempool will be tried once.

Changed Code

To support this change, we introduce BlockLimitsFunctions, which allows the user to specify custom block limits. Currently, there is no choice but to take the limits from the StacksEpoch. Now, users writing custom tools or tests will be able to override these values.

Testing

The new memory analyzer tool is just tested by running it. It's not currently in production.

The changes to introduce BlockLimitsFunctions are tested by the existing tests.

@CLAassistant
Copy link

CLAassistant commented Jan 27, 2022

CLA assistant check
All committers have signed the CLA.

@gregorycoppola gregorycoppola changed the base branch from master to develop January 27, 2022 21:38
@gregorycoppola gregorycoppola linked an issue Jan 27, 2022 that may be closed by this pull request
@codecov
Copy link

codecov bot commented Jan 27, 2022

Codecov Report

Merging #3020 (f352b79) into develop (c6e780a) will increase coverage by 0.11%.
The diff coverage is 27.80%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3020      +/-   ##
===========================================
+ Coverage    82.22%   82.34%   +0.11%     
===========================================
  Files          242      243       +1     
  Lines       195373   195564     +191     
===========================================
+ Hits        160642   161031     +389     
+ Misses       34731    34533     -198     
Impacted Files Coverage Δ
src/chainstate/stacks/mod.rs 75.86% <ø> (ø)
testnet/stacks-node/src/mempool_analyzer.rs 0.73% <0.73%> (ø)
src/clarity_vm/clarity.rs 93.20% <65.78%> (-0.82%) ⬇️
src/chainstate/stacks/db/mod.rs 86.91% <100.00%> (+0.13%) ⬆️
src/chainstate/stacks/miner.rs 93.91% <100.00%> (+0.01%) ⬆️
src/deps/bitcoin/network/serialize.rs 44.00% <0.00%> (-4.00%) ⬇️
src/vm/ast/traits_resolver/mod.rs 94.44% <0.00%> (-3.97%) ⬇️
src/burnchains/bitcoin/mod.rs 37.68% <0.00%> (-1.45%) ⬇️
src/net/atlas/download.rs 82.87% <0.00%> (-1.31%) ⬇️
src/vm/database/key_value_wrapper.rs 96.48% <0.00%> (-0.32%) ⬇️
... and 29 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c6e780a...f352b79. Read the comment docs.

Copy link
Contributor

@pavitthrap pavitthrap left a comment

Choose a reason for hiding this comment

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

Seems to me that this feature can stay in a side branch (as opposed to merging in master)

@@ -601,11 +602,12 @@ impl<'a> StacksMicroblockBuilder<'a> {
/// # Pre-Checks
/// - skip if the `anchor_mode` rules out micro-blocks
/// - skip if 'tx.txid()` is already in `considered`
/// - skip if adding the block would result in a block size bigger than `MAX_EPOCH_SIZE`
/// - skip if adding the block would result in a block size bigger than the block maximum
Copy link
Contributor

Choose a reason for hiding this comment

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

s/the block maximum/the maximum size of a block

@@ -268,9 +286,52 @@ impl ClarityBlockConnection<'_> {
}
}

// Maximum amount of data a leader can send during its epoch (2MB).
const MAX_EPOCH_SIZE: u32 = 2 * 1024 * 1024;
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you redefining this constant here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved the definition here and made it private, so that it would be exposed only through the relevant BlockLimitsFn.

@gregorycoppola
Copy link
Contributor Author

We could keep this in a side branch, but could still consider putting the hooks into build_anchored_block to allow for custom block limits, like "unlimited".

@gregorycoppola
Copy link
Contributor Author

Not checking this in now.

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.

feat: Mempool Analysis Suite
3 participants