Repository navigation
Conversation
A sysio.* contract pays for the actions it is called with and has unlimited CPU, and subjective billing dropped the authorizer's charge once the transaction was in a block, so nothing limited how much CPU one account could use through those contracts. subjective_billing::on_block now reads the billed CPU of each sysio.*-paid action from the block's receipts and adds it to the first authorizer's decaying accumulator, for blocks this node produced and blocks it received. verify_init_subjective_billing then holds the authorizer to subjective-account-cpu-allowed-us plus its own CPU limit per subjective-account-decay-time-minutes, wherever it submits. An explicit sysio.payer, a payer outside sysio.* and the accounts in disable-subjective-account-billing are not charged. The charge is reduced by the block's age, so a syncing node charges old blocks only what is left of them. disable-subjective-sysio-authorizer-billing turns it off. Accounts whose bill has fully decayed are now pruned from the cache. Change-Id: I4617f68ad50a271e37fabece9f1ddf8d0c3d1c62
huangminghuang
left a comment
There was a problem hiding this comment.
Two actionable findings from reviewing the diff and relevant billing paths. Validation: 18 isolated billing tests passed, and targeted checks reproduced both issues described inline. Full integration tests were not run because existing local test libraries failed to link against current headers.
| static account_name billed_authorizer( const action& act, const transaction& trx ) { | ||
| const account_name first_authorizer = act.first_authorizer(); | ||
| return first_authorizer.empty() ? trx.first_authorizer() : first_authorizer; |
There was a problem hiding this comment.
High · performance — Cache the transaction fallback authorizer outside the per-action loops.
transaction::first_authorizer() scans regular actions until one has authorization. A transaction with N authorization-free actions followed by one authorized action therefore makes this helper O(N) for each action, and both on_block and update_billed_cpu_time become O(N^2). This even affects transactions paid entirely by non-sysio contracts, since on_block resolves the authorizer before checking the payer. An optimized local reproduction measured about 0.42 ms for 1,000 actions and 103 ms for 16,000 actions (288 KB, below the default transaction NET limit). These scans run outside the measured action CPU, allowing crafted transactions to impose substantial unmetered work on producing nodes and every block recipient. Compute trx.first_authorizer() once per transaction and reuse it.
There was a problem hiding this comment.
Fixed in 8ee90eb. billed_authorizer takes the transaction's first authorizer as an argument again, and both callers look it up once per transaction: update_billed_cpu_time before its loop, as on master, and bill_sysio_paid_authorizers per receipt. The work is linear in the actions. The actions test now includes a regular action without authorization.
| const uint64_t bill = cpu_us * remaining_window / _expired_accumulator_average_window; | ||
| _account_subjective_bill_cache[authorizer].expired_accumulator.add( | ||
| bill, time_ordinal, _expired_accumulator_average_window ); |
There was a problem hiding this comment.
Medium · bug — Preserve the original decay timeline when applying delayed blocks.
The age discount reduces the initial amount, but add(bill, time_ordinal, full_window) starts a fresh full decay window at receipt time. With a single 1,000 us action at T and a 24-hour window, a live node has no bill at T+24h, while a node applying that block at T+12h still charges 250 us at T+24h and clears it only at T+36h. Thus equally configured nodes can disagree about whether an authorizer is throttled long after catching up. Preserve the remaining lifetime or reconstruct charges on the historical timeline, and test subsequent decay rather than only the initial discounted balance.
There was a problem hiding this comment.
Fixed in 8ee90eb. The charge is now added at the block's time instead of reduced and added at receipt, so a node applying a block late holds the same charge from then on as a node that applied it live. The test compares a late and a live node at W/2, 3W/4 and W. The accumulator keeps a single timeline, so when the account was updated after the block (for example a failure charged at receipt), the block's charge is first decayed to that update.
…horizer once A block applied late charged its age-reduced CPU at receipt time, and that charge then decayed over a full window from there, so a syncing node held it longer than a node that applied the block live. The charge is now dated at the block's time. billed_authorizer took the transaction and rescanned its actions for the fallback authorizer on every action, which is quadratic for a transaction with many actions without authorization. Callers look it up once per transaction again. Change-Id: Ic3e059276426285d75313c84ab53994e44955dce
huangminghuang
left a comment
There was a problem hiding this comment.
Re-reviewed 8ee90eb. Both earlier findings are resolved, and I found no new actionable issues. Validation: 22 isolated C++ tests passed, and transaction_context.cpp compiled successfully. Full integration tests were not run because existing local test libraries are incompatible with current headers.
|
#752 puts entries after the input entries of a receipt's |
Under contract-pays, an action with no explicit
sysio.payeris paid by its contract, and everysysio.*contract has unlimited CPU. Subjective billing charged the authorizer only while the transaction was pending and dropped the charge once it was in a block, and producers did not bill transactions in their own blocks.subjective_billing::on_blocknow charges the billed CPU of eachsysio.*-paid action in the block, read from its receipts, to the action's first authorizer, where it decays oversubjective-account-decay-time-minuteslike a failure. It runs for blocks a node produces and blocks it receives, so every node holds the same charge whichever node the transactions were sent to.verify_init_subjective_billingalready rejects an authorizer oversubjective-account-cpu-allowed-usplus its own CPU limit, so it enforces the charge unchanged.sysio.payer(the payer pays from its objective CPU), a contract outsidesysio.*, and accounts indisable-subjective-account-billing.disable-subjective-sysio-authorizer-billing(defaultfalse) turns the charge off.The work in
on_blockis a pass over the receipts' actions, which are already unpacked in the block. A transaction re-included after a fork switch is charged again.Operators sending
sysio.*actions on a schedule will be charged too; the batch operator option to pay withsysio.payeris #748.