Skip to content

Conversation

@LucasGrasso
Copy link

@LucasGrasso LucasGrasso commented Oct 24, 2025

Summary

Partially Addresses #2653.

  • [ n ] y/n | Does it introduce breaking changes?
  • [ n ] y/n | Is it dependent on a specific version of cargo-contract or pallet-revive?

Expands the TypedEnvBackend trait and its implementations to implement the gas_limit host fn.

Description

Implements the gas_limit host fn in the on-chain enviroment and provides e2e tests for it.

Checklist before requesting a review

  • I have added an entry to CHANGELOG.md
  • I have commented on my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • Any dependent changes have been merged and published in downstream modules
  • Have working tests

@LucasGrasso
Copy link
Author

LucasGrasso commented Oct 24, 2025

@cmichi Ill leave this draft PR so to ask questions in the meantime.

This is what I have right now, and im getting the following error when running e2e tests

---- gas_hostfns::e2e_tests::e2e_gas_limit_works stdout ----
 [==] Building cargo project
 [==] Post processing code

thread 'gas_hostfns::e2e_tests::e2e_gas_limit_works' panicked at /home/USER/Desktop/polkadot/ink/crates/e2e/src/xts.rs:472:35:
decoding `trace_tx` result failed: Could not decode `Option::Some(T)`:
        Could not decode `Trace::Call.0`:
                Could not decode `CallTrace::child_call_count`:
                        Not enough data to fill buffer

I don't really understand the error, what I am missing?
Do I need to copy the output value into a buffer as the other env functions?

@Daanvdplas
Copy link
Contributor

You need to run the ink-node (check out https://use.ink/docs/v6/getting-started/deploy-your-contract). Pop CLI does this however for you!

Please let me know if that works!

@LucasGrasso
Copy link
Author

LucasGrasso commented Oct 24, 2025

Hey Daan, thanks for your message.

On the docs its stated that its automatically ran, despite that I've tried running e2e tests after manually booting the node and with pop test --e2e and in both I've had the same issue.
Also had the same issue with the misc-hostfns e2e tests.

Im running

pop test --e2e integration-tests/internal/gas-hostfns
pop test --e2e integration-tests/internal/misc-hostfns

@Daanvdplas
Copy link
Contributor

What you can also do is use the runtime_only backend like I am doing here: #2686

Then for the e2e tests you don't have to run a node, it is also faster dev cycle.

You might have to add the precompiles in the sandboxed runtime though, but you can do that.

@cmichi
Copy link
Collaborator

cmichi commented Oct 27, 2025

@LucasGrasso The PR looks fine, your error likely comes from using an older ink-node. Can you install the latest ink-node and try again?

cargo install --force --locked --git https://github.com/use-ink/ink-node --branch main

This one has not been released yet, but we use it in the CI of ink/master (which has also not yet been released).

There are some CI failures due to cargo fmt, the easiest thing to fix is if you just run this in the ink! folder:

fd Cargo.toml | xargs -n1 -I{} bash -c "echo {}; cargo +nightly fmt --manifest-path={}" 

@LucasGrasso
Copy link
Author

LucasGrasso commented Oct 27, 2025

Worked with ink-node!
Don`t know why it didn't work with the binary nor with pop cli, probably a versioning issue from my side.

Opening the PR for review. Ran formatting.

@LucasGrasso LucasGrasso marked this pull request as ready for review October 27, 2025 19:50
@cmichi
Copy link
Collaborator

cmichi commented Oct 28, 2025

@LucasGrasso I left some minor comments, but all in all your PR looks great and you got the hang of it!

Also nice that you thought about adding a changelog entry, nearly everyone forgets about that :-).

If you want, feel free to take a look at some of the other host functions in #2653! There's a bunch that follow the same outline.

@LucasGrasso
Copy link
Author

@cmichi applied proposed changes, thanks for your help and review!

If you want, feel free to take a look at some of the other host functions in #2653! There's a bunch that follow the same outline.

I sure will! Would you rather I create one PR per fn or just pack maybe two or three in one PR, I dont want to overwhelm with a huge PR.

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.

3 participants