Page MenuHomePhabricator

redefine virtual transaction size to something useful
ClosedPublic

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

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Commits
rABCddbfa3d06958: redefine virtual transaction size to something useful
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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markblundeberg created this revision.Sat, Jan 11, 09:29
Herald added a reviewer: Restricted Project. · View Herald TranscriptSat, Jan 11, 09:29
markblundeberg planned changes to this revision.Sat, Jan 11, 10:48
markblundeberg edited the summary of this revision. (Show Details)

get rid of sigopscost concept (new parent)

deadalnix accepted this revision.Sat, Jan 11, 16:33
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.Sat, Jan 11, 16:33
markblundeberg added inline comments.Sun, Jan 12, 02:09
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.

rebase, SigOps->SigOpCount