Pool tx weight verification

[antiochp]

So it turns out treating the txpool as "one big tx" for validation purposes meant we were verifying the max weight of the whole transaction pool and checking it did not exceed MAX_BLOCK_WEIGHT...

This is clearly sub-optimal for a bunch of reasons.

Existing verify_weight() logic takes a with_reward bool flag to allow txs and blocks to both be weight verified. A block will have an additional output and kernel to cover the coinbase reward, so txs have to be slightly less weighty to account for this.

max tx weight + coinbase reward = max block weight

This PR introduces a Weighting enum and replaces the with_reward bool flag with the enum.

pub enum Weighting {
	/// Tx represents a tx (max block weight, accounting for additional coinbase reward).
	AsTransaction,
	/// Tx represents a block (max block weight).
	AsBlock,
	/// No max weight limit (skip the weight check).
	NoLimit,
}

So now we can robustly validate the contents of the txpool by treating it as "one big tx" after aggregation, but skipping the weight verification step.

This is all slightly confusing because we -

• validate blocks (and the txpool) as if they were txs
• but then we verify weight against MAX_BLOCK_WEIGHT (as if they were blocks)

So in the context of weight we validate txs as if they were pretending to be blocks.
And blocks as if they were pretending to be txs pretending to be blocks.

We validate the txpool as if it was "one big tx", but for weight purposes we treat it as having no weight limit.

[yeastplume]

I think the approach makes a lot of sense (I was trying to find the issue where we were discussing this, over the possibility that someone could post a transaction with a fee and have no say over what other transactions they get lumped with).

Just thinking through consensus issues. We're relaxing requirements for the block tx, and adding a constraint to individual transactions that wasn't there before. So an earlier client might have rejected a huge individual tx via the block (throwing out other transactions with it), whereas a new client would throw it out at the individual TX level. I'd guess the worst that can happen here is a brief fork?

[antiochp]

We're relaxing requirements for the block tx, and adding a constraint to individual transactions that wasn't there before.

No - this does not change the tx or block validation logic. It just rewrites it to make it more flexible.

The only thing we're affecting is the "can I add this valid tx to the txpool" logic.
And this relaxes the rules - we now allow the txpool to accept more txs and to grow larger than MAX_BLOCK_WEIGHT (which was incorrect behavior).

[ignopeverell]

Just minor comments but a great fix.

[antiochp]

Haven't merged this yet but its good to go.
Just wanted to check that we're good with this going into 1.0.1 before merging.

@ignopeverell

[ignopeverell]

I'm all good with that.