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

tests(app): 🎇 mock consensus delegates, and undelegates, to a validator #4044

Merged
merged 3 commits into from
Mar 20, 2024

Conversation

cratelyn
Copy link
Contributor

@cratelyn cratelyn commented Mar 18, 2024

see #3995.

this test now delegates to, and then undelegates from, a validator.

this does not yet include all of the assertions we would like to
exercise in this test, but it does show that a validator correctly
progresses through the active and defined states.

in further work, we will fill out the other assertions described
in #3995, particularly w.r.t. voting power and validator uptime.

@cratelyn cratelyn added A-testing Area: Relates to testing of Penumbra A-mock-consensus Area: Relates to the mock consensus engine labels Mar 18, 2024
@cratelyn cratelyn added this to the Sprint 2 milestone Mar 18, 2024
@cratelyn cratelyn self-assigned this Mar 18, 2024
@cratelyn
Copy link
Contributor Author

  2024-03-18T20:59:00.777269Z  INFO penumbra_app::server::consensus: deliver_tx failed, e: failed to deliver transaction

Caused by:
    0: check_stateless failed
    1: binding signature failed to verify
    2: Invalid signature.
    at crates/core/app/src/server/consensus.rs:207
    in penumbra_mock_consensus::block::execute with height: 1002, time: 1710795540

i'll tackle this tomorrow, delegating currently fails. i imagine i will need to flesh out more of the details in that transaction.

@cratelyn cratelyn force-pushed the kate/mock-consensus-delegate-to-validator branch from 4628023 to b88730d Compare March 19, 2024 21:19
@cratelyn cratelyn changed the title tests(app): 🎇 more staking component coverage tests(app): 🎇 mock consensus delegates, and undelegates, to a validator Mar 19, 2024
@cratelyn cratelyn marked this pull request as ready for review March 19, 2024 21:20
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note for review: the diff on this looks very strange here, side-by-side view is probably best.

outlines two variables in `Delegate` into methods. these will be used by
our staking test to facilitate the construction of delegation
transactions.
see #3995.

this test now delegates to, and then undelegates from, a validator.

this does not yet include all of the assertions we would like to
exercise in this test, but it does show that a validator correctly
progresses through the active and defined states.

in further work, we will fill out the other assertions described
in #3995, particularly w.r.t. voting power and validator uptime.
@cratelyn cratelyn force-pushed the kate/mock-consensus-delegate-to-validator branch from b88730d to 0796223 Compare March 19, 2024 21:24
@cratelyn cratelyn merged commit 78423e5 into main Mar 20, 2024
6 checks passed
@cratelyn cratelyn deleted the kate/mock-consensus-delegate-to-validator branch March 20, 2024 14:04
amount: self.delegation_amount,
asset_id: self.delegation_token().id(),
});
let stake: Balance = self.unbonded_value().into();
Copy link
Member

Choose a reason for hiding this comment

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

Totally subjective and lgtm, but I have come to appreciate struct literals because they make it immediately apparent what data we are constructing vs. using constructors that add a level of indirection.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agree! this came out of a pairing session with henry, because want to use those same literals in the mock_consensus_staking test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

crates/core/component/stake/src/undelegate.rs Show resolved Hide resolved
@erwanor
Copy link
Member

erwanor commented Mar 20, 2024

Race condition :)

cratelyn added a commit that referenced this pull request Mar 26, 2024
this code was commented out when working on #4044. as of #4070,
(see #4040) we now sign blocks. let's turn this code back on.
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 A-testing Area: Relates to testing of Penumbra
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants