Skip to content

Conversation

@danielvo11
Copy link
Contributor

@danielvo11 danielvo11 commented Dec 15, 2020

  • allow user to config rollbackFunction in AloxideActionHandler
  • reset index state to block number when rollback

Require lecle/demux-js#2 to be merged first

@danielvo11 danielvo11 requested a review from manh-vv December 15, 2020 03:49
@codecov-io
Copy link

codecov-io commented Dec 15, 2020

Codecov Report

Merging #130 (1b0086f) into master (e880000) will increase coverage by 0.04%.
The diff coverage is 97.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #130      +/-   ##
==========================================
+ Coverage   95.64%   95.68%   +0.04%     
==========================================
  Files          72       72              
  Lines        1217     1252      +35     
  Branches      155      162       +7     
==========================================
+ Hits         1164     1198      +34     
- Misses         53       54       +1     
Impacted Files Coverage Δ
packages/demux/src/AloxideActionHandler.ts 98.83% <97.36%> (-1.17%) ⬇️

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 e880000...1b0086f. Read the comment docs.

@danielvo11 danielvo11 force-pushed the feat/#76-implement-roll-back-to branch 3 times, most recently from f136f49 to 5c80bc5 Compare December 15, 2020 10:52
@manh-vv
Copy link
Member

manh-vv commented Dec 23, 2020

Let's implement this feature without introduce breaking change in the demux-js.

  protected async rollbackTo(blockNumber: number): Promise<void> {
    this.log.debug('-- roll back to block number:', blockNumber);
    // - [ ] TODO alter current index state
    // - [ ] TODO revert all change from blockNumber to current blockNumber
    // Aloxide Updater will create a snapshot of data when applying a last irreversible block
    // doing so we can quickly roll back to the last irreversible block state
    // This is only applied to Aloxide data
  }

  public async handleBlock(nextBlock: NextBlock, isReplay: boolean): Promise<number | null> {
    try {
      return super.handleBlock(nextBlock, isReplay);
    } catch (e) {
      if (e instanceof MismatchedBlockHashError) {
        // roll back to the last irreversible blocks
        // - [ ] TODO explain why it must be the last irreversible block
        // - [ ]  what hapen if the rollbackTo took too long to execute
        await this.rollbackTo(this.lastIrreversibleBlockNumber);
        return this.lastIrreversibleBlockNumber + 1;
      }

      throw e;
    }
  }

cc @danielalvess

@danielvo11 danielvo11 force-pushed the feat/#76-implement-roll-back-to branch 5 times, most recently from b7c2de1 to 76508d8 Compare December 29, 2020 04:18
@danielvo11
Copy link
Contributor Author

@manh-vv please help me review again!.

Copy link
Member

@manh-vv manh-vv left a comment

Choose a reason for hiding this comment

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

Please have a direct talk if you have any questions about my comments.

@danielvo11 danielvo11 force-pushed the feat/#76-implement-roll-back-to branch from 76508d8 to 1b0086f Compare January 5, 2021 04:23
Copy link
Member

@manh-vv manh-vv left a comment

Choose a reason for hiding this comment

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

LGTM

@manh-vv manh-vv merged commit 0c43ca9 into master Jan 8, 2021
@manh-vv manh-vv deleted the feat/#76-implement-roll-back-to branch January 8, 2021 09:34
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.

5 participants