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

Adjust create account fee in Sharding #319

Conversation

NIC619
Copy link
Contributor

@NIC619 NIC619 commented Jan 27, 2018

What was wrong?

Since

  1. FlatTrieBackend
  2. Binary Trie

is adopted in Sharding, touch_account doesn't seem to be needed because account's code_hash, balance and nonce now belongs to different trie entries. Also, initializing these attributes to b'' has no effect in a Binary Trie.

How was it fixed?

  • remove touch_account called in ShardingComputation.apply_message solved in Fix transaction execution #313
  • fix gas fee charged for creating a new account(to be discussed, see below)

However since we no longer create a new account like we used to, we should not charge create_gas_fee but instead (1)separately charge for setting code_hash, nonce and balance if not been set before. Or (2)we still charge it but multiply it by x(say 3 since there will be 3 trie entries for one account, but I'm not sure that's all the reason it's charged 25000 gas for creating a new account).

At first thought there doesn't seem to be an easy and clean way to charge separately so I personally would prefer (2).

Cute Animal Picture

put a cute animal picture link inside the parentheses

@NIC619 NIC619 force-pushed the fix_touch_and_create_account_in_flattriebackend branch from f41b494 to e8c0aef Compare January 29, 2018 12:43
@NIC619
Copy link
Contributor Author

NIC619 commented Jan 29, 2018

Rebased and seems like touch_account is removed in #313 . This PR is adjusted to change the fee charged for create_gas_fee.

@NIC619 NIC619 changed the title Remove touch account and adjust create account fee in Sharding Adjust create account fee in Sharding Jan 29, 2018
@vbuterin
Copy link

However since we no longer create a new account like we used to, we should not charge create_gas_fee but instead (1)separately charge for setting code_hash, nonce and balance if not been set before

IMO bad idea. The reason is that the code hash, node and balance still share ~90% of their Merkle tree branch, so it's most practical to consider it as a single item. So I'd prefer keeping the cost of 25000 for state changes that make accounts nonempty for the first time.

@NIC619
Copy link
Contributor Author

NIC619 commented Jan 29, 2018

Then I think this one is good to go.

@NIC619 NIC619 merged commit b35d0f3 into ethereum:sharding Jan 29, 2018
@NIC619 NIC619 deleted the fix_touch_and_create_account_in_flattriebackend branch January 29, 2018 16:17
NIC619 added a commit to NIC619/py-evm that referenced this pull request Feb 2, 2018
hwwhww pushed a commit to hwwhww/py-evm that referenced this pull request Feb 2, 2018
hwwhww pushed a commit to hwwhww/py-evm that referenced this pull request Feb 6, 2018
hwwhww pushed a commit to hwwhww/py-evm that referenced this pull request Feb 18, 2018
hwwhww pushed a commit to hwwhww/py-evm that referenced this pull request Feb 22, 2018
hwwhww pushed a commit to hwwhww/py-evm that referenced this pull request Feb 23, 2018
hwwhww pushed a commit to hwwhww/py-evm that referenced this pull request Mar 2, 2018
hwwhww pushed a commit that referenced this pull request Mar 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants