Page MenuHomePhabricator

redefine virtual transaction size to something useful
ClosedPublic

Authored by markblundeberg on Jan 11 2020, 09:29.

Details

Summary

Currently GetVirtualTransactionSize is a useless placeholder function that
simply returns size, also its comment describes it as having to do with
weight.

Redefine it to be useful: for dense-sigops transactions, it increases above
the actual size, representing the 'excess resources' consumed. Accordingly
the default value for nBytesPerSigOp is increased from 20 to 50, a number
based on actual consensus limits.

This is analogous to what Core does in the mempool sense of virtual size,
but really it's very different since we don't have segwit witness weight
and their choice of sigops constant is too permissive: Core's definition
is max(weight, nsigopscost * 20)/4, i.e., max(bip141size, nsigopscount * 20).

(Note: as of this Diff, nothing in the codebase uses virtual size.)

Depends on D4895 and D4898

Test Plan

ninja check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
virtualsize_def
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 8850
Build 15675: Default Diff Build & Tests
Build 15674: arc lint + arc unit

Event Timeline

deadalnix added inline comments.
src/policy/policy.h
66 ↗(On Diff #15333)

Because this rounded down, this metric *CANNOT* be used for block template construction (unless the result of the computation is exact). It is still appropriate for billing.

This revision is now accepted and ready to land.Jan 11 2020, 16:33
src/policy/policy.h
66 ↗(On Diff #15333)

This is just a default for the user-selected parameter -bytespersigop, whose setting shouldn't affect the validity of created blocks, no matter what value is chosen.