Chain: Remove base_per_transaction_net_usage consensus parameter#519
Conversation
After PR #111 replaced the per-transaction base NET charge with the computed per-action overhead (billable_net_per_action_overhead), this parameter was only read by chain_config::validate() and echoed through SHiP and contract ABIs. A silently-unused on-chain parameter invites confusion -- governance could vote the value and nothing would happen -- so remove it entirely: - chain_config_v0: drop the field, enum id, comparison, logging, FC_REFLECT entry and the id-tagged pack/unpack cases; ids above it shift down by one (PARAMS_COUNT 20 -> 19, packed size 80 -> 76) - config: drop default_base_per_transaction_net_usage and the min_net_usage_delta_between_base_and_max_for_trx validation that existed only to bound it - genesis_state: drop the initializer; the field packed into genesis_state, so the chain_id of every genesis changes (regenesis required) - state_history: drop the field from the ship ABI and the global_property serializer - sysio.system: renumber the set_parameters_packed ids in setparams - renumber the hardcoded parameter ids in the params_test and action_results test contracts - rebuild sysio.bios/sysio.system/params_test/action_results against the matching wire-cdt blockchain_parameters removal - regenerate reference data for the new chain id: deep-mind log, snapshot compatibility files, consensus_blockchain, and the sys-util snapshot info expectations Pairs with the wire-cdt blockchain_parameters removal: the host unpacks the packed blob directly into chain_config_v0, so the CDT struct must match its fc::raw layout field-for-field.
| action_validate_exception, | ||
| "max transaction net usage must be at least {} bytes larger than base net usage per transaction", | ||
| config::min_net_usage_delta_between_base_and_max_for_trx ); | ||
| SYS_ASSERT( context_free_discount_net_usage_den > 0, action_validate_exception, |
There was a problem hiding this comment.
[P1] Preserve a minimum transaction NET limit
Removing the base field also removes the only lower-bound validation for max_transaction_net_usage. A value of zero now passes validation whenever max_block_net_usage is positive. transaction_context then caps every non-read-only transaction at this value and rejects every billable action—including a corrective setparams transaction—leaving the chain recoverable only by hard fork. Please preserve a base-independent minimum transaction NET limit and add rejection tests for zero and undersized values.
There was a problem hiding this comment.
Fixed in 2603982: validate() now enforces a base-independent floor, config::min_max_transaction_net_usage = 10 KiB (matching the old effective bound from the delta check), with the recovery rationale documented on the constant. system_host_tests covers zero and floor-1 (both rejected) and the floor value (accepted).
Review follow-up: removing base_per_transaction_net_usage also removed the delta check that was the only lower bound on max_transaction_net_usage. A zero or tiny value would pass validation and then cap every billable transaction -- including the corrective setparams -- wedging the chain. Enforce a base-independent floor (config::min_max_transaction_net_usage, 10 KiB, matching the old effective bound) and cover zero, floor-1, and floor values with tests.
2 participants
Summary
Removes the
base_per_transaction_net_usagechain config parameter. Since #111 replaced the per-transaction base NET charge with the computed per-action overhead (billable_net_per_action_overhead), the parameter has been dead weight: only read bychain_config::validate()and echoed through SHiP and contract ABIs. A silently-unused consensus parameter invites confusion -- governance could vote the value and nothing would happen.Consensus impact
genesis_state, so the chain id of every genesis changes: this is a regenesis-level change and a running network cannot adopt it in place (the referencesnap_v1chain id is now087244f65e31c0106a58554b8f855e30ae657efb98c6c40348bb14db8bdb3f8e).set_parameters_packed/get_parameters_packedprotocol renumbers: ids above 3 shift down by one,PARAMS_COUNT20 -> 19.sysio.system::setparamsand theparams_test/action_resultstest contracts are renumbered to match.chain_config_v0shrinks 80 -> 76 bytes (system_host_testsstability check updated).chain_config_v0loses the field in both the ABI and theglobal_propertyserializer; old SHiP streams and old snapshots are incompatible, which regenesis covers.Lockstep
set_blockchain_parameters_packedblob directly intochain_config_v0, so CDT'sblockchain_parametersmust match field-for-field.sysio.bios,sysio.system,params_test, andaction_resultsare rebuilt against that change (get/set_blockchain_parametersare compiled into CDT'slibsysio, so the rebuild required a fully rebuilt CDT, not just the patched header).SysioContractTypes.ts) and Cluster-tool: Drop base_per_transaction_net_usage from generated genesis wire-tools-ts#28 (generated genesis + bootstrap doc).Regenerated reference data
Deep-mind log, snapshot compatibility files (
blocks.*,snap_v1.*),consensus_blockchain, and thesys-util snapshot infoexpectations.