Replace the concept of virtual block size, introduce the concept of block cost, and express CBS as a function of it

[NicolasDorier]

The concept of MAX_BLOCK_SIZE can be removed more easily for the next block size HF.
Using MAX_BLOCK_COST make it easier to adapt HF proposals after segwit deployment, because the limited resources will not really be MAX_BLOCK_SIZE anymore.

This will have another advantages for later: The notion of "Block cost" can be later tweaked to include other parameters like SigOp cost.

We can imagine, in the next hardfork, to remove the limitation on the number of SigOps, and just include SigOps as an additional cost parameter in the "Block Cost".

Current rule about sig op could even be coded into GetBlockCost: (pseudocode)

size_t GetBlockCost(const CBlock& block)
{
    // witness data is 4 times less costly than core data, because we can prune it
    size_t coreCost = ::GetSerializeSize(block, SER_NETWORK, 0);
    size_t witCost = ::GetSerializeSize(block, SER_NETWORK, SERIALIZE_TRANSACTION_WITNESS) - coreCost;
    size_t sigCost = GetSigOpCount(block) > MAX_SIG_OPS ? MAX_BLOCK_COST + 1 : 0;
    return witCost + 4 * coreCost + sigCost;
}

[CodeShark]

Subtractions like this make me a little uneasy...unless you bound the result at zero. Although perhaps it's overkill here, assuming SERIALIZE_TRANSACTION_WITNESS always produces a larger result than 0 - but the problem is that a bug in the serialization code could lead to underflow issues which could lead to extremely bizarre behavior.

[CodeShark]

size_t witCost = ::GetSerializeSize(block, SER_NETWORK, SERIALIZE_TRANSACTION_WITNESS);
assert(witCost >= coreCost);
witCost -= coreCost;

[CodeShark]

Actually, an assert would be best

[CodeShark]

size_t is an unsigned integer.
do:
size_t witSize = ::GetSerializeSize(block, SER_NETWORK, SERIALIZE_TRANSACTION_WITNESS);
assert(witSize >= coreSize);
witSize -= coreSize;

[sipa]

How about just using my original formula which has no subtractions?

[CodeShark]

Yes, just multiply it through by 4.

[NicolasDorier]

@CodeShark I made your changes.

@sipa your formula was for calculating the "Virtual Block Size". A concept that I'd like to get away to introduce the concept of "Block cost" which is what should be used as a base for fee calculation for example for a transaction for example. (Virtual Block Size and Core Block Size will be concept that will most likely disappear in the next HF)

If you prefer using the virtual block size, I see no objection to that, but I think I detailed nice reason to have a block cost metric instead. It is both simpler to review, easier to tweak in the future, and easier to explain in the BIP.

For future HF BIP, they should not tweak CBS which won't have a relevant meaning after segwit passed.

[CodeShark]

ACK on the terminology. I'll let sipa speak to the formula clarification.

[NicolasDorier]

@sipa disregard my previous comment I misunderstood what you meant.
What you propose is:

return coreSize * 3 + totalSize;

I'm not really for it, it is less easy to review. But I can change the PR if you insist.

[NicolasDorier]

@sipa @CodeShark I just updated the code to get rid of the substraction and added a comment for review.

[sipa]

My only concern here is things like feerate values that are stored/communicated, as they will become "per kilocost" instead of "per kilobyte".

[NicolasDorier]

As they should be. Since this is the real limiting factor now. And "per kilobyte" is technically true as long as the cost only include segwit and core size.
If you are interested into this PR, I can put it on segwit3.

[sipa]

@NicolasDorier Right, but we want backward compatibility. So when for example a command line option for feerates is given, which used to be intended as per kilobyte, we'd likely want to multiply the passed value by 4 to set the internal per-cost variable.

[NicolasDorier]

Ok I understand what you mean now. This PR does not break the meaning of feerate though. Feerate is still calculated on the size, not on the cost. This is a problem unrelated to this PR.

But indeed worth noting: As miners must maximize the fee/cost to maximize profit, consumers of command line option or RPC will still be dealing with fee/bytes.
This is not an urgent thing to fix, but the concept of feerate will need to be deprecated, since this is not aligned with miner's incentive anymore. This PR does not change this fact.

[sipa]

You can't calculate fee based on size. The optimal strategy for miners is to choose transactions sorted by fee per cost, as cost is the limited resource now. I believe my segnet branches already do that.

[NicolasDorier]

yes, I agree. This PR does not change anything about that.
Do you want I send this PR on segwit3 ?