Skip to content

Conversation

@tzemanovic
Copy link
Collaborator

Describe your changes

Rewrite native VPs to be env-agnostic (WASM or native).

The goal is to:

  • break the dep on vp crate from systems crates that implement their VP to use vp_env instead
  • this will unblock tx & VP ctx and tests refactors #3787 to move WASM Ctx into vp_env so that it can be used directly by crates that depend on vp_env as an implementation of trait VpEnv that can be used for testing without additional deps (same refactor has already been applied in this P for tx_env and tx Ctx)
  • vp_env can then depend on vm to implement VP eval over WASM without running into dep cycle

Checklist before merging

  • If this PR has some consensus breaking changes, I added the corresponding breaking:: labels
    • This will require 2 reviewers to approve the changes

@tzemanovic tzemanovic mentioned this pull request Sep 12, 2024
1 task
@codecov
Copy link

codecov bot commented Sep 12, 2024

Codecov Report

Attention: Patch coverage is 88.52459% with 98 lines in your changes missing coverage. Please review.

Project coverage is 72.70%. Comparing base (d32e856) to head (63d1113).
Report is 18 commits behind head on main.

Files with missing lines Patch % Lines
crates/governance/src/vp/pgf.rs 44.23% 29 Missing ⚠️
crates/node/src/protocol.rs 68.65% 21 Missing ⚠️
crates/shielded_token/src/vp.rs 84.93% 11 Missing ⚠️
crates/governance/src/vp/mod.rs 96.19% 10 Missing ⚠️
crates/ethereum_bridge/src/vp/bridge_pool_vp.rs 94.07% 9 Missing ⚠️
crates/node/src/shell/finalize_block.rs 0.00% 8 Missing ⚠️
crates/ethereum_bridge/src/vp/eth_bridge_vp.rs 94.44% 3 Missing ⚠️
crates/trans_token/src/vp.rs 97.08% 3 Missing ⚠️
crates/vp_env/src/lib.rs 66.66% 2 Missing ⚠️
crates/ethereum_bridge/src/vp/nut_vp.rs 94.44% 1 Missing ⚠️
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3807      +/-   ##
==========================================
+ Coverage   72.69%   72.70%   +0.01%     
==========================================
  Files         338      338              
  Lines      104467   104536      +69     
==========================================
+ Hits        75944    76006      +62     
- Misses      28523    28530       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tzemanovic tzemanovic force-pushed the tomas/env-agnostic-vps branch from 4fc3d95 to 95c58a6 Compare September 12, 2024 09:38
@tzemanovic tzemanovic force-pushed the tomas/env-agnostic-vps branch from 95c58a6 to c812120 Compare September 12, 2024 11:06
@tzemanovic tzemanovic force-pushed the tomas/env-agnostic-vps branch from 86a00c9 to c3fe124 Compare September 12, 2024 11:52
@tzemanovic tzemanovic force-pushed the tomas/env-agnostic-vps branch from c3fe124 to a75ad66 Compare September 12, 2024 15:06
@tzemanovic tzemanovic marked this pull request as ready for review September 13, 2024 07:34
@tzemanovic tzemanovic requested a review from grarco September 13, 2024 07:35
@grarco grarco added the merge Ready to merge - mergifyio bot will add the PR to merge queue when all checks pass label Sep 13, 2024
mergify bot added a commit that referenced this pull request Sep 13, 2024
@mergify mergify bot merged commit 24f9fba into main Sep 13, 2024
@mergify mergify bot deleted the tomas/env-agnostic-vps branch September 13, 2024 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge Ready to merge - mergifyio bot will add the PR to merge queue when all checks pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants