Charge for size and witnesses #791

shargon · 2019-05-29T22:48:53Z

Closes #840

codecov-io · 2019-05-29T22:52:24Z

Codecov Report

Merging #791 into master will increase coverage by 4.83%.
The diff coverage is 61.42%.

@@            Coverage Diff             @@
##           master     #791      +/-   ##
==========================================
+ Coverage   38.73%   43.57%   +4.83%     
==========================================
  Files         176      177       +1     
  Lines       12489    12547      +58     
==========================================
+ Hits         4838     5467     +629     
+ Misses       7651     7080     -571

Impacted Files	Coverage Δ
neo/SmartContract/Native/NativeContract.cs	`85.26% <ø> (ø)`	⬆️
neo/SmartContract/Native/PolicyContract.cs	`100% <ø> (ø)`	⬆️
neo/SmartContract/WitnessWrapper.cs	`0% <0%> (ø)`
neo/Cryptography/Helper.cs	`25.42% <0%> (+8.32%)`	⬆️
neo/SmartContract/InteropService.cs	`20.98% <100%> (+0.17%)`	⬆️
neo/SmartContract/InteropService.NEO.cs	`25.79% <14.28%> (+12.99%)`	⬆️
neo/Network/P2P/Payloads/BlockBase.cs	`69.11% <28.57%> (-7.94%)`	⬇️
neo/SmartContract/Native/Tokens/GasToken.cs	`38.23% <33.33%> (ø)`	⬆️
neo/Network/P2P/Payloads/ConsensusPayload.cs	`76.71% <33.33%> (-8.14%)`	⬇️
neo/Consensus/ConsensusContext.cs	`54.77% <50%> (ø)`	⬆️
... and 30 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87ea7c6...5c1db4e. Read the comment docs.

igormcoelho · 2019-05-30T01:22:43Z

I think I get the idea.. reuse network fee to cover system fee costs too, right?
Is it to avoid a possible "double spending" of the fee?
dont you.prefer the concept of witness/vscript consuming netfee, and script consuming sysfee?

shargon · 2019-05-30T06:11:29Z

Currently you don't pay for verification execution, with thi change you will pay

erikzhang · 2019-05-30T07:15:20Z

Why not using NetworkFee for verification?

shargon · 2019-05-30T07:16:30Z

Why we don't unify both fees in only one?

erikzhang · 2019-05-30T07:54:37Z

sys_fee is distributed to NEO holders. net_fee is distributed to consensus nodes.

vncoelho · 2019-05-30T09:05:16Z

Do you think that fees to CN are important, Erik?
It wouls be nice to burn these fees, like Ripple...kkkkk

erikzhang · 2019-05-30T09:16:38Z

Should we charge both the transaction size and the verification? Or should take the maximum of them?

shargon · 2019-05-30T10:47:48Z

I think that in verification we should pay for both, and in the invocation for the script execution. @vncoelho ripple could create more when they want 😂

shargon · 2019-05-30T10:49:35Z

But for me, only neo holders should receive the gas, CNs doesn't need it because, ussally, they are holders

erikzhang · 2019-05-31T06:08:57Z

So the algorithm is:

Gas = InvocationCost
NetworkFee = VerificationCost + tx.Length * FeePerByte

Is it correct?

Another way is:

Gas = InvocationCost
NetworkFee = max(VerificationCost, tx.Length * FeePerByte)

shargon · 2019-05-31T06:59:09Z

I preffer the first one

shargon · 2019-05-31T08:27:54Z

I think that we should have only one fee field, and CNs take his part from the same field.

igormcoelho · 2019-05-31T14:38:47Z

@erikzhang @shargon I like a lot this direction. Could we consider Turing incomplete design for verification? So we dont even need to charge for length, only verification.

So I propose option 3: xD
Gas = invocation cost
NetworkFee = verification cost (no loops)

If not possible, I go with Erik option 1: netcost = verifcost + transmission cost

I think that we should have only one fee field, and CNs take his part from the same field.

Unified fee system is nice too. verification fee would go to CN, integer invocation fee to claims, and the rest burned?

erikzhang · 2019-06-02T15:10:56Z

We should remove low priority transactions from PolicyContract and MemoryPool.

shargon · 2019-06-02T16:02:49Z

I am worry about receiving Neo in my wallet and not being able to transfer it because i don't have enough gas. Other blockchains use the same token, so they could not have this problem, but we have two

erikzhang · 2019-06-02T16:06:04Z

Don't worry. In NEO 3.0, we will distribute some gas in genesis block.

shargon · 2019-06-02T16:09:55Z

but if one user receive one neo, how much time he need to wait for generate enough gas for transfer it?

shargon · 2019-06-21T11:32:07Z

Done from my side

neo.UnitTests/UT_Transaction.cs

neo/Network/P2P/Payloads/BlockBase.cs

igormcoelho

Trying to make blockbase serialization more readable.

shargon · 2019-06-21T14:15:26Z

Trying to make blockbase serialization more readable.

In other PR please

igormcoelho · 2019-06-21T14:17:57Z

But I'm not understanding it hahaha

neo/Network/P2P/Payloads/BlockBase.cs

igormcoelho

Good work, let's move on!

vncoelho

Water flows, as well as us. aheuahuea
Let's move forward, continue discussions and evolve.

igormcoelho · 2019-06-21T20:35:02Z

shargon · 2019-06-22T08:23:31Z

Merge?

igormcoelho · 2019-06-22T17:52:04Z

Merge, merge, merge... 😂

* fix issue neo-project#791 * follow up * follow up * add example

* Fee= VerificationCost+ApplicationCost * NetworkFee for verification * Clean * Clean * Update * Remove priority from Policy and MemoryPool * Fix CalculateFee * Allow to cancel transactions with more Gas * Revert change We need to test this first * Add Cosigners * some fixes * Update Transaction.cs * format * fix overflow * fix `Wallet.MakeTransaction()` * fix `Transaction.CalculateFees()` * remove contract get from verification * Revert "remove contract get from verification" This reverts commit 6d0dad2. * Fix fee calculation * fix * fix tests * Update MemoryPool.cs * fix tests * UT - Good calculation! * Clean * Revert Nep5Token.cs * Revert conditional * Improve readability * Revert "Improve readability" This reverts commit cd61b98. * Possible fix for unit test * Error verbosity * Add using for ApplicationEngine * Fix unit test * more readable merkleroot test * Sample for multisig contract * Sign ut * Signed! * Same format for single signature unit test * Prevent create unwanted objects * format * Remove `snapshot` from `MakeTransaction()` * Rename * format * using json based NEP6Wallet import * at least using read serializable array * revert last commit

Fee= VerificationCost+ApplicationCost

dd427c7

shargon requested review from igormcoelho and erikzhang May 29, 2019 22:48

shargon mentioned this pull request May 29, 2019

Mode to Disable Jumps and Turing-Completeness neo-project/neo-vm#151

Closed

shargon added 3 commits May 30, 2019 10:58

NetworkFee for verification

fe9a505

Clean

7cda593

Clean

d690f7b

Merge branch 'master' into tx-fee

481f182

Update

1a1247a

erikzhang mentioned this pull request Jun 1, 2019

SVM-2 #784

Closed

Merge branch 'master' into tx-fee

72ad07a

erikzhang added this to the NEO 3.0 milestone Jun 2, 2019

erikzhang added 4 commits June 21, 2019 18:03

format

e484fbe

Remove snapshot from MakeTransaction()

5bbfd02

Rename

63f24fc

format

ff1d71e

erikzhang previously approved these changes Jun 21, 2019

View reviewed changes

erikzhang requested a review from igormcoelho June 21, 2019 10:45

Merge branch 'master' into tx-fee

13ae9a1

igormcoelho requested changes Jun 21, 2019

View reviewed changes

neo.UnitTests/UT_Transaction.cs Outdated Show resolved Hide resolved

using json based NEP6Wallet import

70ccc77

igormcoelho dismissed erikzhang’s stale review via 70ccc77 June 21, 2019 13:51

igormcoelho reviewed Jun 21, 2019

View reviewed changes

neo/Network/P2P/Payloads/BlockBase.cs Show resolved Hide resolved

igormcoelho reviewed Jun 21, 2019

View reviewed changes

neo/Network/P2P/Payloads/BlockBase.cs Show resolved Hide resolved

igormcoelho requested changes Jun 21, 2019

View reviewed changes

at least using read serializable array

dd39152

igormcoelho reviewed Jun 21, 2019

View reviewed changes

neo/Network/P2P/Payloads/BlockBase.cs Outdated Show resolved Hide resolved

revert last commit

ccad415

igormcoelho approved these changes Jun 21, 2019

View reviewed changes

vncoelho approved these changes Jun 21, 2019

View reviewed changes

Merge branch 'master' into tx-fee

5c1db4e

shargon merged commit 90fa9af into neo-project:master Jun 23, 2019

shargon deleted the tx-fee branch June 23, 2019 00:22

Thacryba pushed a commit to simplitech/neo that referenced this pull request Feb 17, 2020

fix issue neo-project#791 (neo-project#793)

d8f8f9a

* fix issue neo-project#791 * follow up * follow up * add example

roman-khimov mentioned this pull request Jul 1, 2020

Drop array wrapper from witnesses for block and consensus payloads #1742

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Charge for size and witnesses #791

Charge for size and witnesses #791

shargon commented May 29, 2019 •

edited by erikzhang

Loading

codecov-io commented May 29, 2019 •

edited

Loading

igormcoelho commented May 30, 2019

shargon commented May 30, 2019

erikzhang commented May 30, 2019

shargon commented May 30, 2019

erikzhang commented May 30, 2019

vncoelho commented May 30, 2019

erikzhang commented May 30, 2019

shargon commented May 30, 2019

shargon commented May 30, 2019 •

edited

Loading

erikzhang commented May 31, 2019 •

edited

Loading

shargon commented May 31, 2019

shargon commented May 31, 2019

igormcoelho commented May 31, 2019 •

edited

Loading

erikzhang commented Jun 2, 2019

shargon commented Jun 2, 2019

erikzhang commented Jun 2, 2019

shargon commented Jun 2, 2019

shargon commented Jun 21, 2019

igormcoelho left a comment

shargon commented Jun 21, 2019 •

edited

Loading

igormcoelho commented Jun 21, 2019

igormcoelho left a comment

vncoelho left a comment

igormcoelho commented Jun 21, 2019

shargon commented Jun 22, 2019

igormcoelho commented Jun 22, 2019

Charge for size and witnesses #791

Charge for size and witnesses #791

Conversation

shargon commented May 29, 2019 • edited by erikzhang Loading

codecov-io commented May 29, 2019 • edited Loading

Codecov Report

igormcoelho commented May 30, 2019

shargon commented May 30, 2019

erikzhang commented May 30, 2019

shargon commented May 30, 2019

erikzhang commented May 30, 2019

vncoelho commented May 30, 2019

erikzhang commented May 30, 2019

shargon commented May 30, 2019

shargon commented May 30, 2019 • edited Loading

erikzhang commented May 31, 2019 • edited Loading

shargon commented May 31, 2019

shargon commented May 31, 2019

igormcoelho commented May 31, 2019 • edited Loading

erikzhang commented Jun 2, 2019

shargon commented Jun 2, 2019

erikzhang commented Jun 2, 2019

shargon commented Jun 2, 2019

shargon commented Jun 21, 2019

igormcoelho left a comment

Choose a reason for hiding this comment

shargon commented Jun 21, 2019 • edited Loading

igormcoelho commented Jun 21, 2019

igormcoelho left a comment

Choose a reason for hiding this comment

vncoelho left a comment

Choose a reason for hiding this comment

igormcoelho commented Jun 21, 2019

shargon commented Jun 22, 2019

igormcoelho commented Jun 22, 2019

shargon commented May 29, 2019 •

edited by erikzhang

Loading

codecov-io commented May 29, 2019 •

edited

Loading

shargon commented May 30, 2019 •

edited

Loading

erikzhang commented May 31, 2019 •

edited

Loading

igormcoelho commented May 31, 2019 •

edited

Loading

shargon commented Jun 21, 2019 •

edited

Loading