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 revert txs and inconsistent state #433

Conversation

jjyr
Copy link
Collaborator

@jjyr jjyr commented Nov 27, 2021

This PR fixes the potential inconsistent state during mem-pool reset.

Issue

Some users report they found txs revert after get the tx receipt.

Context:

The mem-pool reset it's state when receive new l2block, then calculate new state of the next block:

  1. execute l2 withdrawals
  2. execute l2 deposits
  3. execute l2 txs

Unlike withdrawals & txs, the deposits is collected from layer1 CKB's cells, and the count of collected deposits is uncertain everytime.

This 'uncertain' causes the problem, every time we reset the mem-pool the deposits cells may changed, and new created accounts are changed, for example:

The first time mem-pool reset creates 3 accounts:

  1. Deposit A(id: 100)
  2. Deposit B(id: 101)
  3. Contract Account(id: 102)

But when the mem-pool resets again, the accounts are changed:

  1. Deposit A(id: 100)
  2. Deposit B(id: 101)
  3. Deposit C(id: 102)
  4. Contract Account(id: 103)

In example, we collected another new deposit cell, the new deposit changes the id of contract account.

Solution:

This PR introduce a pending_deposits fields in the mem-pool. The new collected deposits cells are stored in the pending_deposits field.

Thus, everytime we reset mem-pool, we use the same pending_deposits to construct next block, so we can get consistent state.

There are two refresh strategy of pending_deposits:

  • safety refresh: when there are no new accounts created in the mem-pool, we are safe to re-collect the deposits cells from CKB. Because this would not change id of accounts.
  • forced refresh: when a deposit is found unavailable in the CKB chain or it's already processed by the l2 block, we are forced to refresh the deposits. (Apply a l2 block with deposits causes forced refresh, but this still a safety operation since the deposits is consist between l2 block and mem-pool state).

@jjyr jjyr requested review from zeroqn and a team November 27, 2021 15:29
@jjyr jjyr force-pushed the fix-invalid-state-by-maintain-pending-deposits branch from 90d4bfe to 76ca32d Compare November 27, 2021 15:31
@jjyr jjyr force-pushed the fix-invalid-state-by-maintain-pending-deposits branch from 76ca32d to 5126e99 Compare November 27, 2021 16:25
@jjyr jjyr changed the title Fix invalid state by maintain pending deposits Fix revert txs and inconsistent state Nov 27, 2021
@jjyr jjyr changed the title Fix revert txs and inconsistent state Fix revert txs caused by inconsistent state Nov 27, 2021
@jjyr jjyr changed the title Fix revert txs caused by inconsistent state Fix revert txs caused and inconsistent state Nov 27, 2021
@jjyr jjyr changed the title Fix revert txs caused and inconsistent state Fix revert txs and inconsistent state Nov 27, 2021
@jjyr jjyr merged commit f88898d into godwokenrises:v0.6.10.x Nov 29, 2021
@jjyr jjyr deleted the fix-invalid-state-by-maintain-pending-deposits branch November 29, 2021 03:39
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.

2 participants