Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NetworkFee calculation method #840

Closed
erikzhang opened this issue Jun 17, 2019 · 42 comments · Fixed by #791
Closed

NetworkFee calculation method #840

erikzhang opened this issue Jun 17, 2019 · 42 comments · Fixed by #791
Assignees
Labels
Design Issue state - Feature accepted but the solution requires a design before being implemented Enhancement Type - Changes that may affect performance, usability or add new features to existing modules. Network-Policy Module - Issues that affect the network-policy like fees, access list and voting

Comments

@erikzhang
Copy link
Member

Currently, our calculation of the NetworkFee of the transaction is divided into two parts: the one is based on the size of the transaction, and the other is based on transaction verification.

So, to calculate fee for a transaction, we need to get the size first. To get the size, we need to fill the Witnesses first. To fill the Witnesses, we need to sign the transaction first. To sign the transaction, we need to fill the NetworkFee first. So that is impossible to do.

Therefore, I propose that we only charge for the part of the transaction other than Witnesses. Because once a transaction is included in a block, the Witnesses field is useless and can be deleted in the future. Therefore, the space occupied by Witnesses can be free.

@neo-project/core What do you think?

@erikzhang erikzhang added the Discussion Initial issue state - proposed but not yet accepted label Jun 17, 2019
@vncoelho
Copy link
Member

Neo Segwit? eaehauhuehauehauea

@erikzhang, I think that I agree. However, for cosigner with different Witnesses we could have a problem and also #835 is an important feature.
Maybe we could have a fixed field in the transaction detailing the amount of witness that will be verified, based on that we could have a fixed price.

@shargon
Copy link
Member

shargon commented Jun 17, 2019

I understand the problem, for me is good because you will pay for the verification execution.

Do you want to include this in #791 ?

@igormcoelho
Copy link
Contributor

igormcoelho commented Jun 17, 2019

Erik, I think it's possible to calculate the network fee, but it will likely create inv script two times.
This is current proposal for tx size:

        public int Size =>
            sizeof(byte) +              //Version
            sizeof(uint) +              //Nonce
            Script.GetVarSize() +       //Script
            Sender.Size +               //Sender
            sizeof(long) +              //Gas
            sizeof(long) +              //NetworkFee
            sizeof(uint) +              //ValidUntilBlock
            Attributes.GetVarSize() +   //Attributes
            Witnesses.GetVarSize();     //Witnesses
  1. First step, fill in this data (note that B is zero array of 32 bytes, and E/G are empty):
----------- TX header (serialize unsigned part) ------------
A) version + nonce + sender + gas (sysfee) + script + validuntil (just fill in regular data)
B) network fee: 32 bytes zero array (will explain later)
C) co-signer1 (attribute):  20 bytes - set to scripthash of witness 1
D) co-signer2 (attribute):  20 bytes - set to scripthash of witness 2
----------- finish TX header ----------------
E) witness 1 (invocation script): empty
F) witness 1 (verification script): regular code
G) witness 2 (invocation script): empty
H) witness 2 (verification script): regular code
  1. Second step, estimate the size of the invocation scripts... how can a wallet do that?
    Examples:
  • standard verification address => size: sround 64 bytes (push signature)
  • multisig verification address => size: around 64 * M (number of expected signers)
    So, a method is needed on Wallet, called EstimatedWitnessInvocationScriptSize() (or a better name please)
    How to proceed with other different verification scripts? Every verification script type must provide its way to estimate the size, and I think it's doable on every case... not that hard. In worst scenario, compute an upper bound on invocation script size is enough.
  1. Third step. After this step, you already have header size, plus witness sizes, so you have tx size. So you can compute first part of networkfee, and store at B (keep it aligned with 32 bytes, to prevent price changes).
    Simulate the execution of verification scripts (you may need to sign temporary header here, on first time), with the real invocation scripts, to estimate the network fee execution cost. This part is the most complicated in my opinion, specially if you have state dependent operations (like ifs that may vary, or any other kinds of dependencies, for example, reading networkfee itself from tx may be problematic, as it will change). In the presented cases, for single and multisig, nothing bad will ever happen. Now you have complete networkfee.

  2. Finally, fill in the real invocation scripts (sign the headers again), and this won't affect network fee anymore. Tx is ready.

=============

Anyway, I still like your proposal Erik. It makes things much simpler to handle. As long as we can archive witnesses in time (forbidding access to Witness.VerificationScript during Application execution, for example), there's no eternal cost being put on the blockchain. Just one question: you mean no charging the witness space, I get that, but also not charging verification costs too? As long as verification is quite constrained, I agree with both.

@erikzhang
Copy link
Member Author

How to proceed with other different verification scripts?

You can't estimate the size of Witness.InvocationScript. Because it may contain arbitrary data.

@igormcoelho
Copy link
Contributor

You can't estimate the size of Witness.InvocationScript. Because it may contain arbitrary data.

Agree.

@erikzhang erikzhang added this to the NEO 3.0 milestone Jun 19, 2019
@igormcoelho
Copy link
Contributor

Ok, if we follow this direction, we need to discuss one thing. Since Witnesses are not supposed to be charged (by space or computation), I strongly suggest we consider a limited computation on Witness computation.
Another thing is, how to efficiently avoid huge witnesses, without any payment form. Just limit number of bytes? This is a small decision that affects everything, we need to think carefully.

@erikzhang
Copy link
Member Author

I have 3 options:

  1. We charge for each byte (including the Witnesses). The advantage is that it can completely prevent attacks. The disadvantage is that the calculation of the fee is very complicated, and it is difficult for other developers to develop for NEO.

  2. We charge for the portion of the transaction other than Witnesses. The advantage is that the calculation of the fee is very simple. But the disadvantage is that someone might send huge Witnesses. In the future, the Witnesses of transactions must be removed from the block.

  3. We restrict Witnesses to only include single-signature contracts or multi-signature contracts. These two known contracts are the most commonly used and can be easily calculated for their size and fees. But the advantage is the loss of flexibility, and Verification can only perform a limited number of functions. But in the future we can expand and support more contract templates.

@neo-project/core

@igormcoelho
Copy link
Contributor

Lets try to put more options Erik...
4. We constrain verification to not allow loops,so it takes maximum flexibility of neovm, in a safe manner. We can make payable header reduced and charge it, but charge also the size of verification scripts on witness (not invocations) and perhaps script too. We can.limit.number of witness and maximum invocation size, since its non paid.

@erikzhang
Copy link
Member Author

@igormcoelho Maybe we can apply this limitation. But it doesn't solve the problem.

@shargon
Copy link
Member

shargon commented Jun 20, 2019

1 is like now on #791, right?

@erikzhang
Copy link
Member Author

1 is like now on #791, right?

Yes.

@igormcoelho
Copy link
Contributor

igormcoelho commented Jun 20, 2019

Maybe we can apply this limitation. But it doesn't solve the problem.

it is part of a complete solution Erik. Limitation allows size of verification be the a limit to the consumption, since there are no loops. I think we can charge verification on network fee (because it can use hashing and ecc), by a simple opcode parsing. Invocation cannot be charged, but we can disable all interops, so only pushes and basic vm logic there, nearly no costs apply to it. We put a max size on invocarions and verifications, although not charging invocations, tx larger than that can be denied. It think this is a reasonable way to proceed.

@shargon
Copy link
Member

shargon commented Jun 20, 2019

I prefer the option one

@vncoelho
Copy link
Member

I prefer option one adjusted to what @igormcoelho is defending.

@igormcoelho
Copy link
Contributor

@erikzhang even if only singlesig and multisig are allowed, will we still be able to invoke deployed contracts on verification? if we do this, we eould need to pay in advance for an arbitrary contract, same issue as before.

@erikzhang
Copy link
Member Author

Invocation cannot be charged, but we can disable all interops, so only pushes and basic vm logic there, nearly no costs apply to it.

I can PUSH and DROP again and again.

@vncoelho
Copy link
Member

vncoelho commented Jun 20, 2019

Let's write about the core ideas of our directions here neo-project/specification#3 and reach an agreement about the re-verification as well...aheuahea

I support more in your direction brother, @igormcoelho:

  • Mempool verification should be fast as a bolt and with auxiliary data structures
  • Verification should allow complex accounts but also could be limited in scope (turing incomplete) for allowing direct calculation of fees

@igormcoelho
Copy link
Contributor

igormcoelho commented Jun 20, 2019

@erikzhang You can do that... this is very cheap, up to the maximum size allowed.
Imagine the max tx size or max witness size? 1024 bytes? 10240 bytes? The impact of several PUSH0 DROP PUSH0 is the same (or less) than a heavy (and useless) PUSHDATAX.
If we quickly parse script before, we will know that someone attached 1000 PUSHES and 1000 DROPS... how much does it cost? 0.00000001 each? so it requires 0.00000001 * 2 * 1000 networkfee. If it pays, let it do his pushes and drops.

@erikzhang
Copy link
Member Author

But you don't charge the size, this method is the same as option 2.

@igormcoelho
Copy link
Contributor

igormcoelho commented Jun 20, 2019

We would charge size Erik, implicitly (or explicitly). I'm proposing to charge only the verification part of witness (not invocation part).
But I see one problem, in the end, it could simply PUSHF, so verification will fail, not by cost, but by logic. I think verification needs limits indeed... not because of verification itself, but because of network. I think network and CN can choose these things: max tx size; max witness size; max invok. witness size; max verif. witness size; max network fee allowed. Verification is easier to adjust in the future, because tx that enter blocks cannot go out.

@erikzhang
Copy link
Member Author

I'm proposing to charge only the verification part of witness (not invocation part).

That's what I said. I can make the invocation part be very long by PUSH and DROP.

@igormcoelho
Copy link
Contributor

Yes, and this would be controlled by max invocation size only. How much size should be allowed on invocation? I hope not much, there's no use for long invocation scripts on witnesses.

@erikzhang
Copy link
Member Author

@vncoelho We are talking about fees, not re-verification. This is another topic.

@erikzhang
Copy link
Member Author

How much size should be allowed on invocation? I hope not much, there's no use for long invocation scripts on witnesses.

Imagine a 99-of-100 multi-signature.

@igormcoelho
Copy link
Contributor

igormcoelho commented Jun 20, 2019

Just was exactly what I calculated now: 100 multisig is 6400 bytes 😂

This is already quite big, and unnecessary for usual cases. If someone wants bigger than that, it is not usual, it can deploy in this case, and fill in signatures one by one, in several transactions, using timelock. This way, it can have a 100000 multisig.

You can allow multisig on verification witness, but lock invocation size. This effectively locks things for "common" uses. After that, deploy can solve anything, no need to do everything on verification.

And to be honest, we can do 99-100 multisig, soon enough I hope, with Schnorr signatures. Fixed invocation size (64 bytes). So, if limiting the whole tx to small sizes, all witnesses are naturally locked too, and technology will self-adjust (or move to deploy).
I mean, for consensus node, we need Schnorr soon to be able to do that very efficiently, if you are thinking on CN uses of verification scripts. It would be very nice to launch Neo 3.0 with Schnorr blocks 🤔

@erikzhang
Copy link
Member Author

Let's step by step. First, we follow the option 1. Then we limit the witness scripts to avoid re-verifications. After that, let's see what's the next.

@igormcoelho
Copy link
Contributor

Agreed.

@igormcoelho
Copy link
Contributor

@lock9 please create an "easy" task: max witness size. It could be added into Policy jsom and filtered by SimplePolicy. Good for newcomers. The only problem is that we dont have SimplePolicy anymore 😂 (so it may become hard... but its worth trying)

@igormcoelho
Copy link
Contributor

A medium level complicated task is to parse script and compute its costs (linear costs). Nothing should be processed, just parse and cost. This is another useful task on this issue. An elaboration over this is to benchmark the cost over CheckSig,CheckMultiSig (with many differebt sizes), and script parsing, for big and small scripts too, just a hunch on computational costs of the approach.

@igormcoelho
Copy link
Contributor

@lock9 I'd also like to continue my PR on jumpless verifications,which was frozen but can be unfrozen now.

@lock9 lock9 added Design Issue state - Feature accepted but the solution requires a design before being implemented Network-Policy Module - Issues that affect the network-policy like fees, access list and voting Enhancement Type - Changes that may affect performance, usability or add new features to existing modules. and removed Discussion Initial issue state - proposed but not yet accepted labels Aug 9, 2019
@erikzhang erikzhang removed this from the NEO 3.0 milestone Dec 6, 2019
@igormcoelho
Copy link
Contributor

@erikzhang one imporrant thing is missing here

@igormcoelho igormcoelho reopened this Dec 6, 2019
@igormcoelho
Copy link
Contributor

its already defined that contracts wont access witness data, ok, good for space pruning. Regarding tx witness execution cost, for new pubkey type it could be free in my opinion, and for other witnesses?
we plan to heavily use witnesses in our dapp, with loops and all... if we adopt again a max gas free charge here on policy, this can.limit usage, and with failed verifications,nothing.charged.
idea of not having loops here is still valid: #837

Otherwise, a valid idea is to charge from Sender, as basic pubkey witness, even if it passes and other witnesses fail. This way we can have loops, secure charging up to max gas funds on verification,and publish BADTX even if other witnesses fail. Limitarion will apply to.script of sender.
what do you think Erik?

@igormcoelho
Copy link
Contributor

@shargon @vncoelho

@erikzhang
Copy link
Member Author

I think we don't need this now.

@vncoelho
Copy link
Member

@igormcoelho, I need to understand better the current situation:

  • As the code is right now we are already discard Witness, right?
  • What is the current limit of the Verification? If the transaction exceeds this limit is something charged?

@shargon
Copy link
Member

shargon commented Sep 15, 2020

It can be closed?

@igormcoelho
Copy link
Contributor

I guess all points should be decided already, right? So it's fine to close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Issue state - Feature accepted but the solution requires a design before being implemented Enhancement Type - Changes that may affect performance, usability or add new features to existing modules. Network-Policy Module - Issues that affect the network-policy like fees, access list and voting
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants