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

Align the block gas limit with the transaction size limit #213

Merged
merged 4 commits into from
Jul 7, 2019

Conversation

semuxgo
Copy link
Contributor

@semuxgo semuxgo commented Jul 7, 2019

To resolve issue #212

This PR intends to align the block gas limit with the transaction size limit, to ensure backwards compatibility.

@semuxgo
Copy link
Contributor Author

semuxgo commented Jul 7, 2019

Now the problem is that this would break compatibility with the testnet.

Copy link
Collaborator

@orogvany orogvany left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could issue new fork flag version. When launching mainnet, tie all the VM related flags into one..

protected Amount minTransactionFee = MILLI_SEM.of(5);
protected Amount minDelegateBurnAmount = SEM.of(1000);
protected long nonVMTransactionGasCost = 21_000L;
protected long nonVMTransactionGasCost = 5_000L;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should prob still be 21,000. Keep gas scaled proportionate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to have 21000 as well but block 64 has more than 2000 transactions https://semux.info/explorer/block/65

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. We still need to figure out a reasonable gas limit. Just one of my sample erc20 creates uses 2 mill gas... Had to bump up cost from old VM testnet for some reason. Not sure if bug or bugfix. So 15mil may be quite low

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@semuxgo I think even 5_000L is not enough. 1 month ago someone submitted 3237 TXs in one block. This one seems to be the biggest block of mainnet so far: https://semux.info/explorer/block/1405616

 height  | transactionsCount
---------+-------------------
 1405616 |              3237
 1399832 |              2941
      66 |              2483
 1405617 |              2338
      65 |              2276
 1405593 |              1734
 1405494 |              1694
  353180 |              1604
  353176 |              1593
   99479 |              1287

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay with bumping the block gas limit to resolve this issue. Currently, Ethereum has a gas limit of ~8mil. Don't want to go too high to avoid any unforeseen troubles.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I'm going to keep the 5000 gas for non-vm-transaction and the rational is that they can be executed more efficiently without launching a virtual machine. This would allow higher throughput without worrying about vm denial-of-service.

@semuxgo
Copy link
Contributor Author

semuxgo commented Jul 7, 2019

I think I'm going to merge this PR and push for the next release candidate. This would require some arrangements:

  • Merge API changes;
  • Make a new release candidate;
  • Upgrade the seed nodes and do a full-sync (they will stop before block 11102);
  • Ask other testnet owners to upgrade and re-sync.

I'd rather not issue another fork just for the testnet.

@semuxgo semuxgo merged commit af50945 into semuxproject:develop Jul 7, 2019
@semuxgo semuxgo changed the title Fix issue #212 Align the block gas limit with the transaction size limit Jul 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants