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: ⭐ use transaction plan in tests #3866

Merged
merged 1 commit into from
Feb 26, 2024

Conversation

@cratelyn cratelyn added C-enhancement Category: an enhancement to the codebase A-mock-consensus Area: Relates to the mock consensus engine labels Feb 22, 2024
@cratelyn cratelyn added this to the Sprint 0 milestone Feb 22, 2024
@cratelyn cratelyn self-assigned this Feb 22, 2024
@cratelyn cratelyn force-pushed the katie/mock-consensus-use-tx-plan branch from 56ca685 to 1171d94 Compare February 23, 2024 02:59
@cratelyn
Copy link
Contributor Author

cratelyn commented Feb 23, 2024

i continued to push this along today.

2024-02-23T02:55:09.755882Z  INFO penumbra_app::server::consensus: deliver_tx failed, e: binding signature failed to verify

Caused by:
Invalid signature.
at crates/core/app/src/server/consensus.rs:210
in penumbra_mock_consensus::abci::deliver_tx
in penumbra_mock_consensus::block::execute with height: 1, time: 1708656909

for tomorrow: i'm currently seeing this error in my logs. noticeably, this doesn't propagate into an error. i'll investigate more tomorrow.

update this happens on main prior to this change. i'm going to merge this, and track down the invalid binding signature in a distinct PR under the umbrella of #3788.

cratelyn added a commit that referenced this pull request Feb 23, 2024
debug logs are rather noisy as a default log level. use info for the
default level, callers can opt in to higher verbosity with `RUST_LOG`.

cherry-picked from #3866.
* #3588
* #3788
* #3857

this rewrites `mock_consensus_can_send_a_spend_action` so that it uses
the `TransactionPlan` API.
@cratelyn cratelyn force-pushed the katie/mock-consensus-use-tx-plan branch from f553338 to 5cf7162 Compare February 26, 2024 15:29
@cratelyn cratelyn marked this pull request as ready for review February 26, 2024 15:36
@cratelyn cratelyn changed the title mock-consensus: ⭐ use transaction plan to add spends mock-consensus: ⭐ use transaction plan in tests Feb 26, 2024
@cratelyn cratelyn merged commit ad7d109 into main Feb 26, 2024
6 checks passed
@cratelyn cratelyn deleted the katie/mock-consensus-use-tx-plan branch February 26, 2024 16:29
cratelyn added a commit that referenced this pull request Feb 27, 2024
fixes #3788.

this branch builds upon the work in #3857 and #3866, filling out the
remaining holes in the transaction plan. this now performs a spend,
using the mock consensus engine.

---------

Co-authored-by: Henry de Valence <[email protected]>
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