fix(api): Improve estimation for gas_per_pubdata_limit

[popzxc]

Fixes the gas_per_pubdata_limit field calculation. Right now, we provide the value we get from deriving the existing batch input, which is most likely lower than limit provided in fee estimation request.
Given that we scale the overall fee, it should be safe to scale the limit as well, as long as we don't go beyond the limit supplied in the tx.

Providing the limit as-is without scaling can be very flaky: transaction risks to be left in the mempool over minor gas_per_pubdata changes.

[perekopskiy]

While it should work as a quick fix I think the problem is more deep.
estimated_fee_scale_factor is set from config var estimate_gas_scale_factor /// The factor by which to scale the gasLimit
we also have gas_price_scale_factor /// The multiplier to use when suggesting gas price

Latter is used in ApiFeeInputProvider but the problem is that provider returns and uses that factor for {fair_l2_gas_price, fair_pubdata_price} while gas_per_pubdata = ceil(fair_pubdata_price / fair_l2_gas_price) so factor is not used for gas_per_pubdata. IMO we should rework fee provider so it's responsible for providing gas_per_pubdata estimate.
Also, not sure about it but it seems if compute_overhead_part=1.0 then gas_price_scale_factor has more effect on fair_l2_gas_price than fair_pubdata_price effectively decreasing gas_per_pubdata

[popzxc]

Agree, but not sure who/when will be able to tackle this in depth.

[slowli]

Dumb question: Do I understand correctly that the changed logic influences zks_estimateFee output only? (There are 2 other uses of gas estimation, eth_estimateGas and zks_estimateL1ToL2Gas, but they both seem to output only Fee.gas_limit.) If so, gas_per_pubdata_limit is overwritten before estimation to a hard-coded value (50,000); is this fine?

zksync-era/core/node/api_server/src/web3/namespaces/zks.rs

Lines 81 to 84 in c156e79

	tx.common_data.fee.max_priority_fee_per_gas = 0u64.into();
	tx.common_data.fee.gas_per_pubdata_limit = U256::from(DEFAULT_L2_TX_GAS_PER_PUBDATA_BYTE);
	self.estimate_fee(tx.into(), block_args, state_override)
	.await

It sort of contradicts "we should not exceed the limit that was provided by the user in the initial request", since the value provided by the user is ignored.

[popzxc]

@slowli

Do I understand correctly that the changed logic influences zks_estimateFee output only?

Yes.

If so, gas_per_pubdata_limit is overwritten before estimation to a hard-coded value (50,000); is this fine?

Huh, that's both fine and not fine. I would expect zks_estimateFee to respect the value if provided, but the reality we're in is that probably nobody actively monitors this value (there is no good tooling and/or documentation for that). So my comment was about the correctness of the implementation, but if we are to discuss the real state of affairs -- it's OK to exceed, since I'm fairly sure that nobody relies on this logic.