Skip to content
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

Rework gas parameters in Broadcaster #1255

Open
raulk opened this issue Jan 20, 2025 · 5 comments
Open

Rework gas parameters in Broadcaster #1255

raulk opened this issue Jan 20, 2025 · 5 comments

Comments

@raulk
Copy link
Contributor

raulk commented Jan 20, 2025

I have been investigating the test failures in #1239.

The latest failure was caused by checkpoints not being signed by validators and not reaching quorum. The root cause was that the Broadcaster component (which validators use to dispatch signatures on-chain) was using static gas_fee_cap and gas_premium values configured in config.toml. By default, these values are 0.

Since we no longer accept a gas_fee_cap below the minimum base fee, we were correctly rejecting these signature attempts. Therefore the checkpoint remained devoid of quorum leading to a timeout in the test.

Potential solutions

  • Solution A: Auto-adjust gas parameters.

    • Setting static gas parameters against a changing gas market simply isn't sound.
    • We could repurpose the configuration settings to determine maximum limits, and the Broadcaster would then calculate the appropriate values on submission.
    • How to determine values:
      • Gas fee cap. We could, for example, attempt to guarantee block inclusion within a specific time bound (e.g. the next block). Ideally we'd include some well-informed slack, but the problem is that we don't have an interface to query the on-chain gas market for the worst case scenario of base fee increase within a specific window. The simplest option is to use the current base fee * some fixed factor.
      • Premium. Ideally we'd reuse the logic of eth_maxPriorityFeePerGas; however it sits in the Eth API layer, and we can't access it from the Broadcaster. Alternatively, we could migrate the premium estimation logic to an ABCI query, where both the Eth API and the Broadcaster could access it.
  • Solution B: Exempt these messages from gas costs.

    • The primary argument is that these are system messages; they're vital for continued operations.
    • However, blindly exempting these messages is risky as it can enable malicious validators to spam the chain for free.
    • Alternatively, we could conditionally exempt the message only if it succeeds, while continuing to charge if it fails. That way, invalid checkpoint signatures would cost, while correct ones would be free.
    • This would require changes to the FVM, but gas cost application is something we wanted to modularise anyway (cc @sanderpick).
    • This could solve another recurring painpoint for genuine validators: the need for validators to have subnet balance only to cover checkpoint signatures.
@cryptoAtwill
Copy link
Contributor

I think solution A is a pain to maintain for validators. Solution B, I think is better.

Another way is to put the checkpoint signing to a separate consensus channel like topdown. Personally I feel the way bottom up signature aggregation on chain is perhaps the easiest to maintain. I think the added benefit of solution B is that it can be used for topdown as well. Just the handling of the effects are different.

@karlem
Copy link
Contributor

karlem commented Jan 20, 2025

I believe an ideal approach is a hybrid of Solutions A and B. First, we would expose a gas_fee_cap and gas_premium estimation via an ABCI query—precisely the kind of function it was designed for. The Broadcaster would then use these estimates to ensure the transaction is priced for timely inclusion. Although validators would still need to front the usual fees, once a transaction is successfully included, the system could provide a full or partial refund specifically for this transaction type. As a result, honest validators must maintain some balance upfront but wouldn’t need to constantly check or replenish it, because the refund mechanism would keep their overall funds intact.

@raulk
Copy link
Contributor Author

raulk commented Jan 27, 2025

@karlem I don't quite understand the value of imposing the need to calculate proper gas fee values (gas fee cap and gas premium), when in the happy path they will never actually be used. Seems a bit wasteful to do that work and to introduce yet another moving part (as @cryptoAtwill points out), when it'll only be used in the uncommon error path.

I tend to concur with @cryptoAtwill that we might as well introduce conditional charging at this point, since @sanderpick also wants modularity at the gas charging level.

To avoid having to deal with dynamic values, we can:

  • As the validator, set the gas fee cap and gas premium to 0 in the transaction.
  • The system identifies this as a system transaction and validates that the validator's balance can cover the base fee * gas limit. Premium is disregarded, since system transactions are anyway prioritised for inclusion (or should be). Note that this check must be performed at block production, not at mpool reception time.
  • If the validator cannot cover the above, the transaction is discarded.
  • When the transaction is executed, if it succeeds, we exempt it from gas charges. If it fails, we charge current base fee * gas limit.

@raulk
Copy link
Contributor Author

raulk commented Jan 27, 2025

@cryptoAtwill Here's a potential work plan.

  1. Fork ref-fvm and introduce gas charging modularity -- need to discuss this in detail but happy for you to come up with a number of approaches for everyone else to review.
  2. Identify these as system transactions, prioritise in the mempool.
  3. At execution time, use the modularity above to deliver the conditionality.

@sanderpick
Copy link
Contributor

Cool, this sounds good to me.

Related, the system level transactions can limit tx throughput because they use the entire block gas limit. If we're going to fork ref-fvm, is there an opportunity to somehow treat these differently? ie, just make the gas limit on the message the exact amount of gas used? Or maybe this would be possible by implementing a custom CallManager: just ignore gas limit if we're using the system actor. There must be some downsides / security implications here though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

No branches or pull requests

4 participants