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

mock-consensus: 💹 generated blocks' height increases #3840

Merged
merged 1 commit into from
Feb 16, 2024

Conversation

cratelyn
Copy link
Contributor

@cratelyn cratelyn commented Feb 16, 2024

this makes changes to the mock consensus engine so that the generated Blocks have monotonically increasing height.

see also:

an example, running the
mock_consensus_can_send_a_sequence_of_empty_blocks test:

; RUST_LOG="penumbra_mock_consensus=info" cargo test --package penumbra-app sequence_of_empty_blocks -- --nocapture
   Compiling penumbra-mock-consensus v0.67.1
   Compiling penumbra-app v0.67.1
    Finished test [unoptimized + debuginfo] target(s) in 9.10s
     Running unittests src/lib.rs (target/debug/deps/penumbra_app-1e86f1775ec8d9a7)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 3 filtered out; finished in 0.00s

     Running tests/mock_consensus.rs (target/debug/deps/mock_consensus-caa27518a071c6af)

running 1 test
  2024-02-16T21:18:58.850646Z  INFO penumbra_mock_consensus::block: sending block
    at crates/test/mock-consensus/src/block.rs:97
    in penumbra_mock_consensus::block::execute with height: 1, time: 1708118338

  2024-02-16T21:18:58.860604Z  INFO penumbra_mock_consensus::block: finished sending block
    at crates/test/mock-consensus/src/block.rs:105
    in penumbra_mock_consensus::block::execute with height: 1, time: 1708118338

  2024-02-16T21:18:58.860725Z  INFO penumbra_mock_consensus::block: sending block
    at crates/test/mock-consensus/src/block.rs:97
    in penumbra_mock_consensus::block::execute with height: 2, time: 1708118338

  2024-02-16T21:18:58.869760Z  INFO penumbra_mock_consensus::block: finished sending block
    at crates/test/mock-consensus/src/block.rs:105
    in penumbra_mock_consensus::block::execute with height: 2, time: 1708118338

  2024-02-16T21:18:58.869883Z  INFO penumbra_mock_consensus::block: sending block
    at crates/test/mock-consensus/src/block.rs:97
    in penumbra_mock_consensus::block::execute with height: 3, time: 1708118338

  2024-02-16T21:18:58.879037Z  INFO penumbra_mock_consensus::block: finished sending block
    at crates/test/mock-consensus/src/block.rs:105
    in penumbra_mock_consensus::block::execute with height: 3, time: 1708118338

  2024-02-16T21:18:58.879143Z  INFO penumbra_mock_consensus::block: sending block
    at crates/test/mock-consensus/src/block.rs:97
    in penumbra_mock_consensus::block::execute with height: 4, time: 1708118338

  2024-02-16T21:18:58.888524Z  INFO penumbra_mock_consensus::block: finished sending block
    at crates/test/mock-consensus/src/block.rs:105
    in penumbra_mock_consensus::block::execute with height: 4, time: 1708118338

  2024-02-16T21:18:58.888659Z  INFO penumbra_mock_consensus::block: sending block
    at crates/test/mock-consensus/src/block.rs:97
    in penumbra_mock_consensus::block::execute with height: 5, time: 1708118338

  2024-02-16T21:18:58.898337Z  INFO penumbra_mock_consensus::block: finished sending block
    at crates/test/mock-consensus/src/block.rs:105
    in penumbra_mock_consensus::block::execute with height: 5, time: 1708118338

  2024-02-16T21:18:58.898486Z  INFO penumbra_mock_consensus::block: sending block
    at crates/test/mock-consensus/src/block.rs:97
    in penumbra_mock_consensus::block::execute with height: 6, time: 1708118338

  2024-02-16T21:18:58.908190Z  INFO penumbra_mock_consensus::block: finished sending block
    at crates/test/mock-consensus/src/block.rs:105
    in penumbra_mock_consensus::block::execute with height: 6, time: 1708118338

  2024-02-16T21:18:58.908325Z  INFO penumbra_mock_consensus::block: sending block
    at crates/test/mock-consensus/src/block.rs:97
    in penumbra_mock_consensus::block::execute with height: 7, time: 1708118338

  2024-02-16T21:18:58.917988Z  INFO penumbra_mock_consensus::block: finished sending block
    at crates/test/mock-consensus/src/block.rs:105
    in penumbra_mock_consensus::block::execute with height: 7, time: 1708118338

  2024-02-16T21:18:58.918116Z  INFO penumbra_mock_consensus::block: sending block
    at crates/test/mock-consensus/src/block.rs:97
    in penumbra_mock_consensus::block::execute with height: 8, time: 1708118338

  2024-02-16T21:18:58.927903Z  INFO penumbra_mock_consensus::block: finished sending block
    at crates/test/mock-consensus/src/block.rs:105
    in penumbra_mock_consensus::block::execute with height: 8, time: 1708118338

test mock_consensus_can_send_a_sequence_of_empty_blocks ... ok

@cratelyn cratelyn added C-enhancement Category: an enhancement to the codebase A-mock-consensus Area: Relates to the mock consensus engine labels Feb 16, 2024
@cratelyn cratelyn added this to the Sprint 0 milestone Feb 16, 2024
@cratelyn cratelyn self-assigned this Feb 16, 2024
this makes changes to the mock consensus engine so that the generated
`Block`s have monotonically increasing height.

see also:
* #3588
* #3788

an example, running the
`mock_consensus_can_send_a_sequence_of_empty_blocks` test:

```
; RUST_LOG="penumbra_mock_consensus=info" cargo test --package penumbra-app sequence_of_empty_blocks -- --nocapture
   Compiling penumbra-mock-consensus v0.67.1
   Compiling penumbra-app v0.67.1
    Finished test [unoptimized + debuginfo] target(s) in 9.10s
     Running unittests src/lib.rs (target/debug/deps/penumbra_app-1e86f1775ec8d9a7)

running 0 tests

test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 3 filtered out; finished in 0.00s

     Running tests/mock_consensus.rs (target/debug/deps/mock_consensus-caa27518a071c6af)

running 1 test
  2024-02-16T21:18:58.850646Z  INFO penumbra_mock_consensus::block: sending block
    at crates/test/mock-consensus/src/block.rs:97
    in penumbra_mock_consensus::block::execute with height: 1, time: 1708118338

  2024-02-16T21:18:58.860604Z  INFO penumbra_mock_consensus::block: finished sending block
    at crates/test/mock-consensus/src/block.rs:105
    in penumbra_mock_consensus::block::execute with height: 1, time: 1708118338

  2024-02-16T21:18:58.860725Z  INFO penumbra_mock_consensus::block: sending block
    at crates/test/mock-consensus/src/block.rs:97
    in penumbra_mock_consensus::block::execute with height: 2, time: 1708118338

  2024-02-16T21:18:58.869760Z  INFO penumbra_mock_consensus::block: finished sending block
    at crates/test/mock-consensus/src/block.rs:105
    in penumbra_mock_consensus::block::execute with height: 2, time: 1708118338

  2024-02-16T21:18:58.869883Z  INFO penumbra_mock_consensus::block: sending block
    at crates/test/mock-consensus/src/block.rs:97
    in penumbra_mock_consensus::block::execute with height: 3, time: 1708118338

  2024-02-16T21:18:58.879037Z  INFO penumbra_mock_consensus::block: finished sending block
    at crates/test/mock-consensus/src/block.rs:105
    in penumbra_mock_consensus::block::execute with height: 3, time: 1708118338

  2024-02-16T21:18:58.879143Z  INFO penumbra_mock_consensus::block: sending block
    at crates/test/mock-consensus/src/block.rs:97
    in penumbra_mock_consensus::block::execute with height: 4, time: 1708118338

  2024-02-16T21:18:58.888524Z  INFO penumbra_mock_consensus::block: finished sending block
    at crates/test/mock-consensus/src/block.rs:105
    in penumbra_mock_consensus::block::execute with height: 4, time: 1708118338

  2024-02-16T21:18:58.888659Z  INFO penumbra_mock_consensus::block: sending block
    at crates/test/mock-consensus/src/block.rs:97
    in penumbra_mock_consensus::block::execute with height: 5, time: 1708118338

  2024-02-16T21:18:58.898337Z  INFO penumbra_mock_consensus::block: finished sending block
    at crates/test/mock-consensus/src/block.rs:105
    in penumbra_mock_consensus::block::execute with height: 5, time: 1708118338

  2024-02-16T21:18:58.898486Z  INFO penumbra_mock_consensus::block: sending block
    at crates/test/mock-consensus/src/block.rs:97
    in penumbra_mock_consensus::block::execute with height: 6, time: 1708118338

  2024-02-16T21:18:58.908190Z  INFO penumbra_mock_consensus::block: finished sending block
    at crates/test/mock-consensus/src/block.rs:105
    in penumbra_mock_consensus::block::execute with height: 6, time: 1708118338

  2024-02-16T21:18:58.908325Z  INFO penumbra_mock_consensus::block: sending block
    at crates/test/mock-consensus/src/block.rs:97
    in penumbra_mock_consensus::block::execute with height: 7, time: 1708118338

  2024-02-16T21:18:58.917988Z  INFO penumbra_mock_consensus::block: finished sending block
    at crates/test/mock-consensus/src/block.rs:105
    in penumbra_mock_consensus::block::execute with height: 7, time: 1708118338

  2024-02-16T21:18:58.918116Z  INFO penumbra_mock_consensus::block: sending block
    at crates/test/mock-consensus/src/block.rs:97
    in penumbra_mock_consensus::block::execute with height: 8, time: 1708118338

  2024-02-16T21:18:58.927903Z  INFO penumbra_mock_consensus::block: finished sending block
    at crates/test/mock-consensus/src/block.rs:105
    in penumbra_mock_consensus::block::execute with height: 8, time: 1708118338

test mock_consensus_can_send_a_sequence_of_empty_blocks ... ok
```
@cratelyn cratelyn force-pushed the katie/mock-consensus-height-increases branch from 5eb6361 to 1104dcd Compare February 16, 2024 21:44
@cratelyn cratelyn marked this pull request as ready for review February 16, 2024 21:57
@cratelyn
Copy link
Contributor Author

merging this opportunistically, because changes are all within penumbra-mock-consensus.

@cratelyn cratelyn merged commit d08d8f2 into main Feb 16, 2024
6 checks passed
@cratelyn cratelyn deleted the katie/mock-consensus-height-increases branch February 16, 2024 22:03
Comment on lines +77 to +78
.with_data(vec![])
.with_evidence(List::new(Vec::new()))
Copy link
Member

Choose a reason for hiding this comment

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

These don't need to be set, right? Wondering if this is an artefact of the dev process, as a (potential) user it's confusing to see invocations of methods with default parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a great catch, thanks. #3848 makes the evidence optional, using an empty list if none is provided.

cratelyn added a commit that referenced this pull request Feb 20, 2024
`tendermint::evidence::List` implements `std::default::Default`. so,
let's use that instead of mandating that an empty evidence list be
provided each time that we build a new block.

#3840 (comment)

Reference: #3588
Reference: #3792
Reference: #3840
Co-Authored-By: Henry de Valence <hdevalence@penumbralabs.xyz>
cratelyn added a commit that referenced this pull request Feb 20, 2024
`tendermint::evidence::List` implements `std::default::Default`. so,
let's use that instead of mandating that an empty evidence list be
provided each time that we build a new block.

#3840 (comment)

Reference: #3588
Reference: #3792
Reference: #3840
Co-Authored-By: Henry de Valence <hdevalence@penumbralabs.xyz>
cratelyn added a commit that referenced this pull request Feb 20, 2024
`tendermint::evidence::List` implements `std::default::Default`. so,
let's use that instead of mandating that an empty evidence list be
provided each time that we build a new block.


#3840 (comment)

Reference: #3588
Reference: #3792
Reference: #3840

Co-authored-by: Henry de Valence <hdevalence@penumbralabs.xyz>
cratelyn added a commit that referenced this pull request Mar 13, 2024
this patches the mock consensus `TestNode::end_block` method so that the
height of these requests does not stay at 1.

this is needed for staking tests, see #3995.

* #3588
* #4001
* #3995
* #3840
cratelyn added a commit that referenced this pull request Mar 15, 2024
this patches the mock consensus `TestNode::end_block` method so that the
height of these requests does not stay at 1.

this is needed for staking tests, see #3995.

* #3588
* #4001
* #3995
* #3840
cratelyn added a commit that referenced this pull request Mar 15, 2024
this patches the mock consensus `TestNode::end_block` method so that the
height of these requests does not stay at 1.

this is needed for staking tests, see #3995.

* #3588
* #4001
* #3995
* #3840
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-mock-consensus Area: Relates to the mock consensus engine C-enhancement Category: an enhancement to the codebase
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants