Skip to content

fix: include PoV base cost estimation for EthereumRuntimeAPI::create()#472

Closed
gonzamontiel wants to merge 27 commits into
mainfrom
fix/frontier-add-pov-to-create
Closed

fix: include PoV base cost estimation for EthereumRuntimeAPI::create()#472
gonzamontiel wants to merge 27 commits into
mainfrom
fix/frontier-add-pov-to-create

Conversation

@gonzamontiel
Copy link
Copy Markdown
Contributor

@gonzamontiel gonzamontiel commented Mar 11, 2026

Summary

In the EthereumRuntimeAPI::create() API, the runtime now passes a proof-size base cost into Frontier’s runner so that PoV validation accounts for the encoded transaction size, not only the gas-derived weight. Before the parameter was set to None.

  • Estimated transaction length is computed for an EIP1559-style create: base 190 bytes (from, value, gas_limit, nonce, action, chain_id, signature) plus data.len(), optional max_fee_per_gas / max_priority_fee_per_gas (32 bytes each when present), and encoded access_list length.
  • Conditional passing: when the converted weight has proof_size() > 0, the runtime passes (weight_limit, proof_size_base_cost) into Runner::create(); otherwise it passes (None, None) so behavior is unchanged when proof size is not used.
  • Applied change in mainnet, stagenet, and testnet runtimes (operator/runtime/*/src/lib.rs)

stiiifff and others added 17 commits February 27, 2026 08:52
When `send_rewards_message` fails at era end (bridge paused, queue full,
etc.), tokens have already been minted but the message to EigenLayer is
lost. This adds automatic retry and a governance escape hatch.

Storage: ring buffer (StorageMap + head/tail pointers, capacity 64) tracks
failed eras with their timestamp and scaled inflation amount.

Retry logic (on_initialize): processes one entry per block. On success the
entry is removed. On failure the entry is moved to the back of the queue
so a single stuck era does not block retries for subsequent ones. Expired
entries (reward points pruned) are discarded automatically.

Governance extrinsic: `retry_unsent_reward_era` gated by configurable
`GovernanceOrigin` allows manual retry of a specific era.

Cleanup: `on_era_start` proactively removes all unsent entries whose
reward points have been pruned (idx <= era_index_to_delete).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Match the IdentityForceOrigin pattern so the retry extrinsic can be
called by either Root or the GeneralAdmin governance origin.

Also add missing EnsureOrigin import in benchmarking.rs required when
compiling with runtime-benchmarks feature.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
## Summary

Remove unused dependency `pallet-staking`.


## What changes

* Remove `pallet-staking` from the Cargo.toml file
* Update Cargo.lock

Co-authored-by: Steve Degosserie <723552+stiiifff@users.noreply.github.com>
New update to StorageHub containing only a simple bugfix. No changes
required in DataHaven

Co-authored-by: undercover-cactus <lola@moonsonglabs.com>
…ical filtering, and liveness E2E test (#447)

Introduces typed offence classification, a linear Perbill-to-WAD
conversion for EigenLayer slashing, historical offence filtering, and a
new E2E test proving end-to-end liveness detection through
`pallet_im_online`.

---

New `OffenceKind` enum classifies consensus offences:
- `LivenessOffence` — missed heartbeats (ImOnline)
- `BabeEquivocation` — double block production
- `GrandpaEquivocation` — double finality votes
- `BeefyEquivocation` — double BEEFY votes / fork voting / future block
voting
- `Custom(BoundedVec<u8, 256>)` — manual / governance slashes

Each variant carries a human-readable description string through the
Snowbridge message to EigenLayer's
`DatahavenServiceManager.slashValidatorsOperator()`.

Generic wrapper around `ReportOffence` wired for BABE, GRANDPA, BEEFY,
and ImOnline in all three runtimes:

1. **Filters historical offences** — discards reports whose session
predates the bonding period, using `BondedEras` storage (analogous to
`FilterHistoricalOffences` in `pallet_staking`, but adapted to this
pallet's own era tracking).
2. **Tags offence kind** — stores the `OffenceKind` in
`PendingOffenceKind` double-map `(SessionIndex, ValidatorId)` before
delegating to `pallet_offences`. The `on_offence` handler reads it via
`take()` in the same block.
3. **Cleans up on failure** — removes stale `PendingOffenceKind` entries
if the inner reporter returns an error (e.g. duplicate report),
preventing them from leaking into unrelated future offences.

Each offence type in Substrate defines its own
`slash_fraction(offenders_count)` returning a `Perbill`:

| Offence | Formula | Typical range |
|---|---|---|
| **BABE equivocation** | `min((3k/n)^2, 1)` | 1 offender / 100
validators: ~0.09%; 1/2: capped to 100% |
| **GRANDPA equivocation** | `min((3k/n)^2, 1)` | Same as BABE |
| **BEEFY double-vote** | `min((3k/n)^2, 1)` | Same as BABE/GRANDPA |
| **BEEFY fork/future voting** | Fixed `50%` | Always 50% |
| **ImOnline liveness** | `min(3*(k - floor(n/10) - 1)/n, 1) * 7%` | 10%
or fewer offline: **0%**; ~33% offline: ~5%; ~43% offline: 7% (max) |

Where `k` = number of concurrent offenders, `n` = validator set size.

**Key behavior for small validator sets (E2E):** With n=2, the ImOnline
threshold is `floor(2/10) + 1 = 1`. A single offender (`k=1`) fails
`checked_sub(1)` giving `Perbill(0)`. This means no `Slashes` storage
entry is created (since `compute_slash` returns `None` when the new
fraction doesn't exceed the prior slash), but the `SlashReported` event
is still emitted, proving the full detection pipeline works.

The Substrate `Perbill` is linearly mapped to a WAD value capped by
`MaxSlashWad`:

```
WAD = perbill.deconstruct() * MaxSlashWad / 1_000_000_000
```

- `MaxSlashWad` default: **5e16** (= 5% in WAD format, where 1e18 =
100%)
- Governance-changeable dynamic runtime parameter (codec index 46)
- `Perbill(100%)` maps to exactly `MaxSlashWad` (the cap)
- `Perbill(0%)` maps to 0 (no slash sent to EigenLayer)

| Scenario | Substrate Perbill | WAD sent to EigenLayer | EigenLayer % |
|---|---|---|---|
| BABE equivocation (1 of 100 validators) | ~0.09% | ~4.5e13 | ~0.0045%
|
| BABE equivocation (1 of 2 validators) | 100% (capped) | 5e16 | 5%
(max) |
| BEEFY fork voting | 50% | 2.5e16 | 2.5% |
| ImOnline liveness (1 of 2 offline) | 0% | 0 (no slash) | 0% |
| ImOnline liveness (~33% of large set offline) | ~5% | ~2.5e15 | ~0.25%
|
| Manual `force_inject_slash` at 20% | 20% | 1e16 | 1% |
| Manual `force_inject_slash` at 100% | 100% | 5e16 | 5% (max) |

The same WAD value is applied uniformly to all configured strategies via
the `SlashingRequest` struct sent through Snowbridge to
`DatahavenServiceManager.slashValidatorsOperator()`.

New test scenario (`should detect and slash an unresponsive validator`)
validates the full liveness detection pipeline:

1. Pauses bob's Docker container (preserving GRANDPA state via `docker
pause`)
2. Waits 200s (>= 2 full sessions) for `pallet_im_online` to detect
missed heartbeats
3. Unpauses bob to restore GRANDPA finality (2/2 validators needed)
4. Polls for `SlashReported` event (not `Slashes` storage — see slash
fraction note above)
5. Verifies the event confirms the full pipeline: `pallet_im_online ->
EquivocationReportWrapper -> pallet_offences -> on_offence`

The test uses `try/finally` to always unpause bob, `{ at: "best" }`
queries for non-finalized chain state during the pause, and drains prior
`SlashReported` events before starting.

- **10 new unit tests**: `PendingOffenceKind` double-map semantics,
session isolation, wrapper historical filtering, error cleanup, WAD
conversion (100%, 50%, 0%), offence kind description propagation
- **New mock infrastructure**: `MockInnerReporter`, `MockOffence`,
`MockOkOutboundQueue` with slash data capture
- **E2E**: Updated `force_inject_slash` test to use `offence_kind` enum,
new liveness detection test

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Gonza Montiel <gonzamontiel@users.noreply.github.com>
Co-authored-by: undercover-cactus <lola@moonsonglabs.com>
## Summary
- fixes the untrusted CI failure in `e2e-tests / E2E Tests with Kurtosis
Ethereum Network`
- keeps validator-set-submitter startup actionable by avoiding test-only
contract config imports during container startup
- improves submitter readiness diagnostics by capturing both stdout and
stderr from container logs and making streamed log matching robust to
chunked UTF-8 output
- reduces validator-set-submitter Docker build time in CI by building
from `test/` and adding a tight `test/.dockerignore`
- makes local arm64 E2E runs use a native local Snowbridge relayer image
instead of forcing `linux/amd64` emulation
- auto-builds the local relayer image when needed for `:local` tags

## Why
The original failing untrusted test started as a submitter container
startup problem, but the branch now also addresses a second timeout path
that showed up while debugging:
- the submitter image was being built from the repository root with a
large Docker context, which made the `validator-set-update` suite spend
most of its hook timeout budget inside `docker build`
- on Apple Silicon, forcing `datahavenxyz/snowbridge-relay:latest`
through `linux/amd64` caused `generate-beacon-checkpoint` to segfault
during local runs

These changes make the submitter failure actionable, cut the CI Docker
build context down substantially, and keep local E2E runs reliable on
arm64.

## Validation
- `cd test && bun fmt`
- `cd test && bun x tsc --noEmit`
- `bun test e2e/suites/validator-set-update.test.ts --timeout 900000`
- `cd test && docker build -f tools/validator-set-submitter/Dockerfile
-t datahavenxyz/validator-set-submitter:local .`
… the operator node (#428)

This PR rename the `rewardsAgentOrigin`, `rewardsMessageOrigin`, etc...
into a less specific less now that the Snowbrige Agent is also being
used to relay slashing messages.

This PR also have a fix to register the Agent address instead of the
operator node address to check the sender of the message. Without this
fix we could never relay rewards or execute slashing because we would
get an error regarding the message.

* Removing the prefix `rewards` everytime we were refering the
snowbridge agent (to clarify that the agent is not only being used by
the reward feature)
* Fix the deployment script to register the `agentAddress` as the
required sender for relaying substrate message

[ ] ~~Rename `onlyRewardsInitiator` and `rewardsInitiator` in the
`DatahavenServiceManager.sol ` for something that would include
slashing~~ This should be done in another PR.
[x] Check the Testnet Deploy script to make sure it is using the agent
address

---------

Co-authored-by: Ahmad Kaouk <56095276+ahmadkaouk@users.noreply.github.com>
@gonzamontiel gonzamontiel requested review from a team as code owners March 11, 2026 14:07
@gonzamontiel gonzamontiel changed the title fix: include PoV base cost estimation in create() fix: include PoV base cost estimation for EthereumRuntimeAPI::create() Mar 11, 2026
@gonzamontiel gonzamontiel added D2-notlive PR doesn't change runtime code (so can't be audited) D3-trivial PR contains trivial changes in a runtime directory that do not require an audit not-breaking Does not need to be mentioned in breaking changes B0-silent Changes should not be mentioned in any release notes and removed D3-trivial PR contains trivial changes in a runtime directory that do not require an audit labels Mar 12, 2026
Copy link
Copy Markdown
Contributor

@ahmadkaouk ahmadkaouk left a comment

Choose a reason for hiding this comment

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

Can you clarify the concrete problem this fixes for DataHaven?

DataHaven is a solo chain, and in our runtimes we explicitly do not account for PoV today (GasLimitPovSizeRatio = 0). So as far as I can tell, gas_to_weight(...).proof_size() stays 0, and this new branch still resolves to (None, None), meaning it does not change behavior for us currently. I’m not convinced this should go in as-is.

@gonzamontiel
Copy link
Copy Markdown
Contributor Author

Can you clarify the concrete problem this fixes for DataHaven?

DataHaven is a solo chain, and in our runtimes we explicitly do not account for PoV today (GasLimitPovSizeRatio = 0). So as far as I can tell, gas_to_weight(...).proof_size() stays 0, and this new branch still resolves to (None, None), meaning it does not change behavior for us currently. I’m not convinced this should go in as-is.

It's part of the work of closing the audit issues. I partially agree, but even if still this resolves to 0 for the time being, it doesn't need to be hardcoded in the configuration.

gonzamontiel and others added 5 commits March 12, 2026 14:10
eth_estimateGas now applies proof_size_base_cost and a proof-size cap,
so the runner reserves PoV for the extrinsic and limits execution to
the remaining budget. Estimates can be lower than before because they
are now bounded by this PoV limit instead of unbounded; update moonwall
expectations (contract creation and Incrementor create) to the new
values.
@gonzamontiel
Copy link
Copy Markdown
Contributor Author

Can you clarify the concrete problem this fixes for DataHaven?
DataHaven is a solo chain, and in our runtimes we explicitly do not account for PoV today (GasLimitPovSizeRatio = 0). So as far as I can tell, gas_to_weight(...).proof_size() stays 0, and this new branch still resolves to (None, None), meaning it does not change behavior for us currently. I’m not convinced this should go in as-is.

It's part of the work of closing the audit issues. I partially agree, but even if still this resolves to 0 for the time being, it doesn't need to be hardcoded in the configuration.

@ahmadkaouk do you mind merging this? Otherwise we can leave a comment explaining why we don't charge PoV currently.

@stiiifff
Copy link
Copy Markdown
Contributor

Blocking issues

  1. The new branch never fires — proof_size() is always 0.
  • All three runtimes set GasLimitPovSizeRatio = 0 (operator/runtime/{mainnet,stagenet,testnet}/src/configs/mod.rs:1024/1021/1024).
  • WeightPerGas = Weight::from_parts(WEIGHT_PER_GAS, 0)proof_size component is 0 per gas unit
    (configs/mod.rs:1019/1016/1019).
  • So gas_to_weight(gas, true).proof_size() is always 0, and the new conditional resolves to (None, None) in 100% of
    cases. The encoded-length arithmetic is computed and discarded.
  1. Subtle behavior regression despite the "no-op" appearance.
  • Before: Some(weight_limit), None → runner enforced a ref_time budget derived from gas_limit.
  • After: None, None → runner runs with no weight limit.
  • This likely explains the gas estimate drop in test-gas-contract-creation.ts (210541156082) and
    test-gas-estimation-contracts.ts (255341172902). These changes are a side effect of removing the ref_time limit,
    not of the PoV accounting the PR title advertises. Worth confirming this is intentional — if so, the PR description
    and title should be rewritten accordingly.
  1. Asymmetric fix: call() is not updated.
  • fn call() at mainnet/src/lib.rs:1094 still passes Some(weight_limit), None with the same issue the PR claims to fix.
    Frontier's upstream template applies PoV base cost to both call and create.

@gonzamontiel
Copy link
Copy Markdown
Contributor Author

I would say we can close this one, there's no big reason to continue working on this / fixing the misbehavior mentioned above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B0-silent Changes should not be mentioned in any release notes D2-notlive PR doesn't change runtime code (so can't be audited) not-breaking Does not need to be mentioned in breaking changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants