Transaction size plugin

[belane]

Plugin to filter Transactions by size. It does not allow large free Tx and requires a proportional fee for large Tx (current values are indicative).

• Not Allow free TX bigger than MaxFreeTransactionSize.
• Not Allow TX bigger than MaxTransactionSize.
• For TX bigger than TransactionExtraSize require proportional fee.

[erikzhang]

Why create a new project instead of merging it into SimplePolicy?

[belane]

You're right, I'm gonna merge it with SimplePolicy.

[erikzhang]

Please review this first: neo-project/neo#381

[RavenXce]

Can we set MaxFreeTransactionSize to 400 bytes instead? That will cover a 2-signature transaction.

[belane]

Can we set MaxFreeTransactionSize to 400 bytes instead? That will cover a 2-signature transaction.

Do you think we need to increase to 400 free Tx? most of normal Tx fit in 200-300 bytes.

[RavenXce]

Can we set MaxFreeTransactionSize to 400 bytes instead? That will cover a 2-signature transaction.

Do you think we need to increase to 400 free Tx? most of normal Tx fit in 200-300 bytes.

Once two signatures are required, it is slightly above 300 bytes. I think it is pretty common and we use that quite a lot.

[RavenXce]

Actually thinking about it, is there a point for MaxFreeTransactionSize? It seems TransactionExtraSize would be sufficient to prevent useless large transactions.

[belane]

@RavenXce I've increased MaxFreeTransactionSize to 400.

The approach is:

1. Allow Transactions without fee below MaxFreeTransactionSize in order to maintain free Tx on Neo but without it can cause any troubles.
2. Require 'any' fee for the rest Tx, between MaxFreeTransactionSize and TransactionExtraSize.
3. Require proportional fee for large transactions.

Current values are not very restrictive, so they can be revised in the future if needed.

I think the change can be merged if you agree @erikzhang.

[RavenXce]

It seems that only requiring any fee for MaxFreeTransactionSize to TransactionExtraSize serves to be more of an annoyance than anything else, as only 1 sat of gas is forced to be paid, which is insignificant.

Users who want fast transactions will already be paying an appropriate amount of fees, and spammers can still increase the blockchain size with very little cost - so nothing much is improved within that size range.

Also I'm concerned about putting out minimum gas fees in general without lots of warnings and announcements to developers and the NEO community. A policy for consensus nodes to not include certain transactions should be treated as non-backward compatible change. Some contracts may not have been built with fee payment in mind, and may not allow withdrawals with fees, for example:

https://github.com/neo-ngd/CGAS-Contract/blob/master/NeoContract/CGAS.cs#L46
https://github.com/ConjurTech/switcheo/blob/v1/switcheo/BrokerContract.cs#L208

[erikzhang]

Agree with @RavenXce, we can remove TransactionExtraSize and use MaxFreeTransactionSize.

[belane]

@RavenXce I'm totally agree that any change should be put in knowledge of all parties involved.

As your and @erikzhang suggestion, removed TransactionExtraSize window and now we require proportional fee for any Tx over MaxFreeTransactionSize.

[erikzhang]

@jsolman Can we merge this PR now?

[RavenXce]

@erikzhang I think this will cause some problems on Switcheo DEX contracts. I went through the transactions and now saw that txns with 2 inputs and 2 output can exceed 400 bytes. If we need to merge this into mainnet can we increase the MaxFreeTransactionSize further first?

https://neotracker.io/tx/e33587a7474386305877220168713f2cff9ff21eaea0cd74caf2083a4803b195

[erikzhang]

Do you think 512 is a good value for MaxFreeTransactionSize?

[RavenXce]

I think to be safe we can start with 2048 or 1024. We can then lower it further if spammers are abusing the allowed size. On Switcheo's end, the max inputs allowed will be limited.

[vncoelho]

Hi, @erikzhang,
In a talk with @belane it was verified that, in the last commit, a minor detail was not included.

return free.Take(Settings.Default.MaxTransactionsPerBlock - non_free.Count() - 1).Concat(non_free);

This last part ensures more space for free transaction (when available).
What do you think about it?

The trade-off is to let this space being used for transaction that are spam or not useful. Anyway..

[belane]

Two small changes:

• Remove FilterForBlock_Policy1, as it was private and not used anymore here .
• Use tx_list also on non_free.

We can discuss later if allow free TXs to fill the block is a good idea, I agree with that, but is not the propose of this PR.

If we all agree, It's ready to go.

[vncoelho]

I also agree, @belane, it is not the purpose of this PR.

Perhaps it can move forward.

[vncoelho]

@belane, one last thing that we could consider.
Currently it is:
if (tx.NetworkFee < Fixed8.FromDecimal(fee)) return false;
In this case, just to send a Fee tx you automatically have some more Transaction size.

Maybe it should be:
if ( tx.NetworkFee < (Fixed8.FromDecimal(fee) + MinFeeThresholds) return false;

But maybe it may become "quite complex" unnecessarily, because then comes another question about what is the part used for priority and what is the part used for size.... aeuahuehaea

Edited: This was already discussed above: #10 (comment)
"Only size over MaxFreeTransactionSize would be charged. So with 0.001 fee you can send a TX up to 1124 bytes."

I think are you all in a good line of reasoning, simple and direct.
Congratulations for this PR.

[shargon]

good catch @belane

[erikzhang]

Remove FilterForBlock_Policy1, as it was private and not used anymore here .

I would prefer to keep FilterForBlock_Policy1 so that we can use this code directly if we want to change the strategy in the future.

[belane]

@erikzhang reverted FilterForBlock_Policy1

[vncoelho]

@belane, let's create a variable in the .config for setting the desired policy.

[erikzhang]

Waiting for Neo 2.9.2