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

Fix: Improve the withdrawal packaging performance #701

Merged
merged 11 commits into from
May 26, 2022

Conversation

jjyr
Copy link
Collaborator

@jjyr jjyr commented May 24, 2022

  • Fix a bug of withdrawal packaging. Before the fix, Godwoken skip packaging withdrawals every two blocks. Now, it packages withdrawals in every block.
  • Increase maximum deposits & withdraws of each block from 50 to 100(100 depostis + 100 withdrawals costs ~ 240_000_000 cycles).
  • Increase the maximum cycles of layer1 submission transaction to 350_000_000 (1/10 of a CKB block).
  • Improve the logging of block-produce

@jjyr jjyr requested review from keroro520, blckngm and zeroqn May 24, 2022 05:13
@keroro520
Copy link
Contributor

ACK the three items:

  • Increase maximum deposits & withdraws of each block from 50 to 100.
  • Increase the maximum cycles of layer1 submission transaction to 350_000_000 (1/10 of a CKB block).
  • Improve the logging of block-produce

I do not quite understand how bug 6596222 can be triggered. Can this bugfix be moved to another PR?

keroro520
keroro520 previously approved these changes May 24, 2022
Copy link
Contributor

@keroro520 keroro520 left a comment

Choose a reason for hiding this comment

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

LGTM

crates/config/src/config.rs Show resolved Hide resolved
@jjyr
Copy link
Collaborator Author

jjyr commented May 24, 2022

ACK the three items:

  • Increase maximum deposits & withdraws of each block from 50 to 100.
  • Increase the maximum cycles of layer1 submission transaction to 350_000_000 (1/10 of a CKB block).
  • Improve the logging of block-produce

I do not quite understand how bug 6596222 can be triggered. Can this bugfix be moved to another PR?

Updated the test case.

@jjyr jjyr requested review from keroro520 and blckngm May 24, 2022 10:19
blckngm
blckngm previously approved these changes May 24, 2022
@zeroqn

This comment was marked as outdated.

@gw-bot

This comment was marked as outdated.

keroro520
keroro520 previously approved these changes May 24, 2022
@jjyr

This comment was marked as outdated.

@gw-bot

This comment was marked as outdated.

@jjyr jjyr dismissed stale reviews from keroro520 and blckngm via 1691df5 May 25, 2022 08:52
@jjyr jjyr force-pushed the logging-produce-block-time branch from 72aac35 to 1691df5 Compare May 25, 2022 08:52
@jjyr

This comment was marked as outdated.

@gw-bot

This comment was marked as outdated.

@jjyr jjyr requested review from keroro520 and blckngm May 25, 2022 09:45
@jjyr

This comment was marked as outdated.

@gw-bot

This comment was marked as outdated.

@jjyr jjyr force-pushed the logging-produce-block-time branch from 1691df5 to f9896af Compare May 25, 2022 11:31
@jjyr
Copy link
Collaborator Author

jjyr commented May 25, 2022

/itest
kicker: compatibility-changes
web3: compatibility-breaking-changes

@gw-bot

This comment was marked as outdated.

1 similar comment
@gw-bot

This comment was marked as outdated.

zeroqn
zeroqn previously approved these changes May 26, 2022
@jjyr jjyr merged commit 87cf28e into godwokenrises:develop May 26, 2022
@jjyr jjyr deleted the logging-produce-block-time branch May 26, 2022 04:08
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.

4 participants