Skip to content

Implement linear fees (a + bx) for aggregate instructions #79

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

nagisa
Copy link
Collaborator

@nagisa nagisa commented May 14, 2025

This has adjusted the finite-wasm analysis and instrumentation to collect fees not as constant values for each operation, but in form of a + bx. However, using linear fees extensively is not recommended, as fees that have b ≠ 0 are not mergeable and will incur a significant instrumentation overhead.

As a future change it may be possible to still merge some instances where b ≠ 0, but since a lot of care will be necessary to make sure that x stays the same, merging logic will need to be somewhat more complicated than it currently is.

Even then, since all of the bulk_memory operations for which linear fees are useful are trapping merging across the instruction is not applicable at all.

This has required somewhat extensive changes to the interpreter we use in our specification as well as to our test runner to allow for situations where the aggregate instruction may trap in the middle of its execution. In particular, say, memory.copy may trap after processing 5 elements (thus charging 5 gas according to the reference interpreter,) but at instrumentation level we only have an instruction-level granularity available, so we must pre-charge gas assuming the entire operation would succeed. This means that the actual gas consumption observed from the interpreter can be anywhere between gas_before_aggregate <= gas_before_aggregate + steps if a trap occurs.

Recommended review strategy is to look at the "implement linear fees for aggregate instructions" in isolation and only skim the 2nd commit to verify instrumentation changes appear sane. Reminder that users aren't required to use instrument.rs module in production and neither does nearcore (though not for wasmtime...) -- they can get by with just analysis results and introduce instrumentation directly into the generated machine code.

Based on top of #78
Fixes #49

@nagisa nagisa force-pushed the nagisa/implements-linear-fees branch from 665b282 to 80989c0 Compare May 15, 2025 13:04
Comment on lines +258 to +263
pub(crate) fn checked_add(self, other: Fee) -> Option<Self> {
Some(Self {
linear: (self.linear == 0 && other.linear == 0).then_some(other.linear)?,
constant: self.constant.checked_add(other.constant)?,
})
}
Copy link
Collaborator Author

@nagisa nagisa May 15, 2025

Choose a reason for hiding this comment

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

Double-checked this and this is fine. Whether instrumentation point can be merged or not is primarily determined by the InstrumentationKind. In past code whether gas can be added was only important for handling of possible overflow.

In principle this could be relaxed somewhat to allow for the 1st instrumentation point with linear fee to merge in any further constant fees into itself, but that makes this function not commutative, and I don't want to review all of the implications of that at this point in time, so we won't allow merging in any case where fees are non-constant for now.

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.

gas: Change the Aggregate instrumentation kind to represent cost as a*x + b
1 participant