Skip to content

piecrust: remove zero-cost host query blanket impl - #491

Merged
d-sonuga merged 1 commit into
mainfrom
dangerous-default-host-query-price
Jul 6, 2026
Merged

piecrust: remove zero-cost host query blanket impl#491
d-sonuga merged 1 commit into
mainfrom
dangerous-default-host-query-price

Conversation

@d-sonuga

@d-sonuga d-sonuga commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Removes the “zero-cost” blanket HostQuery implementation for Fn(&mut [u8], u32) -> u32, making host-query pricing explicit and preventing accidental zero-gas host queries in the Piecrust VM API.

Changes:

  • Removed the blanket HostQuery impl for host query closures from piecrust/src/vm.rs.
  • Updated host-query tests to wrap function-based queries in an explicit ZeroCostHostQuery adapter.
  • Added a CHANGELOG.md entry documenting the removal.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
piecrust/tests/host.rs Adds an explicit ZeroCostHostQuery wrapper and updates test registrations accordingly.
piecrust/src/vm.rs Removes the blanket HostQuery implementation for Fn(&mut [u8], u32) -> u32.
piecrust/CHANGELOG.md Documents the removal under Unreleased → Removed.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread piecrust/src/vm.rs Outdated
@d-sonuga
d-sonuga force-pushed the dangerous-default-host-query-price branch from bb44311 to a7c8337 Compare July 3, 2026 14:56
@d-sonuga
d-sonuga force-pushed the dangerous-default-host-query-price branch from a7c8337 to 33cfb03 Compare July 3, 2026 14:58

@moCello moCello left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

APPROVE — Clean, well-scoped removal of a zero-gas footgun. The test wrapper preserves behavior, all in-tree registrations are updated, and downstream (rusk/vm) already uses explicit HostQuery impls, so nothing breaks. One optional nit: capitalize "Remove" in the commit subject to match the repo convention.

@d-sonuga

d-sonuga commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

capitalize "Remove" in the commit subject to match the repo convention.

There is no repo convention that says that. Over 90% of commits with a "piecrust: " prefix does not have capitalization.

@HDauven HDauven left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@d-sonuga
d-sonuga merged commit ca44653 into main Jul 6, 2026
4 checks passed
@d-sonuga
d-sonuga deleted the dangerous-default-host-query-price branch July 6, 2026 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants