[ENG-1190] fix(fees): compensate v1 compute_budget-field mass on devnet - #38
[ENG-1190] fix(fees): compensate v1 compute_budget-field mass on devnet#38zelenevn wants to merge 2 commits into
Conversation
The node's `transaction_input_estimated_serialized_size` counts a 2-byte `compute_budget` field per input for `version >= 1`, but the wallet's serialized-size helper omits it. The wallet therefore undercounts compute mass by `2 * mass_per_tx_byte` per v1 input. The shortfall scales with the input count and, past a few hundred inputs, pushes the estimate under kaspad's non-standard relay-fee floor — the devnet L2 loadgen contract deploy was rejected `9890100 < 9906600` (compute mass 99066), exactly `2 * inputs - payload_len` short. Rather than branch the mass math on network identity, model the two v1 compute-mass compensations as an explicit `MassCompensations` policy resolved once per network: - Extract a network-agnostic `compute_mass_for_unsigned_consensus_transaction` free function taking `&MassCompensations`; the generator method is a thin wrapper over it. The math never inspects the network. - `MassCompensations::for_network` is the single seam mapping network to enabled compensations: the pre-existing `v1_script_mass` term stays on for all networks; the new `v1_compute_budget_field` term is enabled on devnet only, leaving mainnet/testnet fee sizing byte-for-byte unchanged. - Both terms only ever apply to v1 (non-native subnetwork) transactions; v0 native payments are untouched on every network. Adding a future compensation is a new policy field plus one `for_network` line, not another network conditional in the calculation. Tests: `for_network` mapping across all four network types, and the opt-in behavior of the compute_budget-field term (adds exactly `COMPUTE_BUDGET_FIELD_SIZE * mass_per_tx_byte` per v1 input, no effect on v0). 51 daemon unit tests pass.
pycckuu
left a comment
There was a problem hiding this comment.
Code looks good, just a couple of suggestions.
|
|
||
| #[derive(Clone, Copy, Debug, PartialEq, Eq)] | ||
| struct MassCompensations { | ||
| v1_script_mass: bool, |
There was a problem hiding this comment.
Issue: Replacing the documented workaround with two undocumented flags loses the lifecycle of each compensation. v1_script_mass and v1_compute_budget_field cover independent upstream omissions that can be fixed separately, while the existing upstream canary only detects that the aggregate v0/v1 result changed and cannot identify which flag became stale. With the dependency following upstream master, that ambiguity can turn a future fix into an accidental overcount.
Suggestion: Add field-level documentation naming the exact upstream omission and removal condition for each flag, and split the upstream canary so script-mass support and compute-budget byte accounting are detected independently.
| } | ||
|
|
||
| #[test] | ||
| fn v1_compute_budget_field_compensation_is_opt_in() { |
There was a problem hiding this comment.
Issue: This regression test compares the helper with its flag disabled and enabled, then derives the expected delta from the same COMPUTE_BUDGET_FIELD_SIZE constant used by production. It can pass even if that constant or formula drifts from kaspad, and it does not exercise the network policy, generator wrapper, and final fee-mass path that failed in production.
Suggestion: Add a node-parity regression using a signed, non-native v1 transaction under devnet parameters, preferably with enough inputs to reproduce the original fee-floor case, and assert that the wallet's final noncontextual fee mass matches the canonical consensus mass calculation.
Address review feedback on the devnet v1 compute-budget-field compensation. Each MassCompensations flag now carries field-level documentation naming the exact upstream omission it corrects and the canary that retires it: - v1_script_mass: the missing per-input compute_budget script-mass term in MassCalculator::calc_compute_mass_for_client_transaction_input. - v1_compute_budget_field: the missing 2-byte compute_budget field in transaction_input_serialized_byte_size. Split the single upstream canary into two independent tests so a future upstream fix to one term cannot silently strand the other: - upstream_v1_script_mass_still_omitted varies only compute_budget (equal serialized size), isolating the script-mass term. - upstream_v1_compute_budget_field_still_omitted compares v0/v1 at zero compute_budget, isolating the serialized-size byte term. Add a node-parity regression that builds a signed, non-native v1 transaction with 500 inputs under devnet parameters and asserts the wallet's compute mass equals the canonical kaspa_consensus_core::mass::MassCalculator result. The expected value comes from consensus rather than the local COMPUTE_BUDGET_FIELD_SIZE constant, so any drift in kaspad's byte or script-mass accounting fails the test.
5816eb6 to
1205923
Compare
The wallet undercounts compute mass for v1 (non-native subnetwork / lane) transactions, causing kaspad to reject high-input-count lane payloads as non-standard. This adds the missing per-input mass term, modeled as an explicit per-network compensation policy so the fee math never branches on network identity.
Problem
kaspad's transaction_input_estimated_serialized_size counts a 2-byte compute_budget field per input for version >= 1, but the wallet's serialized-size helper omits it. The wallet therefore undercounts compute mass by 2 * mass_per_tx_byte per v1 input. The shortfall scales with input count and, past a few hundred inputs, drops the estimate below kaspad's non-standard relay-fee floor.
Observed on the devnet L2 loadgen run: the BatchTransfer contract deployment was rejected with
transaction has 9890100 fees which is under the required amount of 9906600 for compute mass 99066
— short by exactly 2 * inputs - payload_len. No load could start.
Solution
The mass kaspad charges is network-independent (a function of ConsensusParams + tx version), so the fix keeps the calculation network-agnostic and expresses the rollout decision as policy:
Adding a future compensation is a new policy field plus one for_network line — not another network conditional inside the calculation.
Backward compatibility
mainnet/testnet/simnet resolve v1_compute_budget_field: false, so their fee sizing is byte-for-byte unchanged. Only devnet sees the new term.
Testing