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.

Sign up for GitHub

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

Jump to bottom

feat: 📝 implement cometstub crate (work-in-progress) #3597

Closed
wants to merge 2 commits into from
Closed

feat: 📝 implement cometstub crate (work-in-progress) #3597

wants to merge 2 commits into from

Conversation

cratelyn
Copy link
Contributor

@cratelyn cratelyn commented Jan 9, 2024

see #3588.

/!\ ------------------------------------------------------- /!\
/!\ NOTE: this is a rolling work-in-progress.               /!\
/!\ this branch will be force-pushed frequently until it is /!\
/!\ ready for review. proceed accordingly!                  /!\
/!\ ------------------------------------------------------- /!\

@cratelyn cratelyn self-assigned this Jan 9, 2024
@cratelyn cratelyn mentioned this pull request Jan 9, 2024
Closed
@cratelyn cratelyn mentioned this pull request Jan 9, 2024
Closed
@TalDerei
Copy link
Collaborator

@cratelyn I pushed a fix to correctly handle loading the lazy proving keys. You were missing a dev dependency

penumbra-proof-params = { path = "../../crypto/proof-params", features = [
    "bundled-proving-keys",
    "download-proving-keys",
] }

without which wasn't calling try_load before attempting to dereference the keys.

@cratelyn
feat: 📝 implement cometstub crate (work-in-progress)
eab617a 
this commit introduces a new library, in `crates/test/`. this library
contains a mock implementation of cometbft, for use in cargo integration
tests.

* this does NOT add the `penumbra-cometstub` crate to the list of
  crates included in the rust documentation in
  `deployments/scripts/rust-docs`. the `penumbra-tct-property-test`
  crate was also not included in that list at the time of writing.

/!\ ------------------------------------------------------- /!\
/!\ NOTE: this is a rolling work-in-progress.               /!\
/!\ this branch will be force-pushed frequently until it is /!\
/!\ ready for review. proceed accordingly!                  /!\
/!\ ------------------------------------------------------- /!\

NOTE 24-01-02: i have brought over a test from
`core/app/src/tests/spend.rs`. currently, it panics because proving keys
could not be loaded. printlns and panics have been left to point out
how far we can get through that test before failing. track down the core
of this issue next week.

use this command to see it break:

```
cargo watch -Bc -x 'test -p penumbra-cometstub spend_happy_path'
```
@cratelyn
🍐 misc: api notes from pair programming
e5ada47 
these are api notes outlining the ideal surface for our mock
consensus engine, driven by a pair programming session.

see: #3588 (comment)

TODO: this should be cleaned up later.
@cratelyn cratelyn added C-enhancement Category: an enhancement to the codebase A-mock-consensus Area: Relates to the mock consensus engine labels Feb 8, 2024
@cratelyn
Copy link
Contributor Author

cratelyn commented Feb 8, 2024

closing this; this exploratory work was carried forward in #3781, and other forthcoming pr's. see #3588 for more information on this line of work.

@cratelyn cratelyn closed this Feb 8, 2024
@cratelyn cratelyn mentioned this pull request Mar 1, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@cratelyn cratelyn

Labels
A-mock-consensus Area: Relates to the mock consensus engine C-enhancement Category: an enhancement to the codebase
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cratelyn @TalDerei