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: tx-pool snapshot consistency #2984

Merged
merged 5 commits into from
Sep 9, 2021

Conversation

zhangsoledad
Copy link
Member

@zhangsoledad zhangsoledad commented Aug 26, 2021

What problem does this PR solve?

Previously, tx-pool has a potential issue, some code gets snapshots directly from the ArcSwap container, which means those snapshots represent the newest chain, it may be inconsistent with tx-pool owned snapshot.

What is changed and how it works?

Remove interface that can get snapshot from swap container.
https://github.com/nervosnetwork/ckb/pull/2984/files#diff-fb91da4f07a6ef121b8913771dd0cf067853b6a3b41eaa96fd242881b71b5f32L659-L664

Check List

Tests

Release note

Title Only: Include only the PR title in the release note.

@zhangsoledad zhangsoledad force-pushed the zhangsoledad/fix-template branch from bd42250 to 52d3727 Compare August 26, 2021 08:26
@zhangsoledad zhangsoledad force-pushed the zhangsoledad/fix-template branch from 52d3727 to 55a5dcb Compare September 1, 2021 09:54
@zhangsoledad zhangsoledad force-pushed the zhangsoledad/fix-template branch from 55a5dcb to 250baf3 Compare September 2, 2021 04:59
@zhangsoledad zhangsoledad marked this pull request as ready for review September 2, 2021 13:08
@zhangsoledad zhangsoledad requested a review from a team as a code owner September 2, 2021 13:08
@zhangsoledad zhangsoledad force-pushed the zhangsoledad/fix-template branch from a87b64f to dedb3f1 Compare September 2, 2021 13:41
@chanhsu001
Copy link
Contributor

i' not qulified to review this pr for i'm not familiar with this part. Could change to other one to reivew?
keroro520 is on vacation, i don't want to block the review.

@driftluo driftluo requested review from quake and yangby-cryptape and removed request for chanhsu001 September 6, 2021 08:58
@driftluo driftluo added the s:waiting-on-reviewers Status: Waiting for Review label Sep 8, 2021
@zhangsoledad
Copy link
Member Author

bors r=yangby-cryptape,driftluo

@bors
Copy link
Contributor

bors bot commented Sep 9, 2021

Build succeeded:

@bors bors bot merged commit 05d530f into nervosnetwork:develop Sep 9, 2021
@driftluo driftluo removed the s:waiting-on-reviewers Status: Waiting for Review label Sep 9, 2021
@zhangsoledad zhangsoledad deleted the zhangsoledad/fix-template branch September 9, 2021 18:27
bors bot added a commit that referenced this pull request Oct 2, 2021
3068: fix: block-template test r=quake,driftluo a=zhangsoledad

<!--
Thank you for contributing to nervosnetwork/ckb!

If you haven't already, please read [CONTRIBUTING](https://github.com/nervosnetwork/ckb/blob/develop/CONTRIBUTING.md) document.

If you're unsure about anything, just ask; somebody should be along to answer within a day or two.

PR Title Format:
1. module [, module2, module3]: what's changed
2. *: what's changed
-->

### What problem does this PR solve?

Problem Summary: 
Since #2984, if tx-pool does not sync with chain yet, a blank template will return by request.  this may lead to some block-template tests failed randomly.

### What is changed and how it works?

What's Changed:  Make block-template tests block on status sync.

### Related changes

- PR to update `owner/repo`:
- Need to cherry-pick to the release branch

### Check List <!--REMOVE the items that are not applicable-->

Tests <!-- At least one of them must be included. -->

- Unit test

### Release note <!-- Choose from None, Title Only and Note. Bugfixes or new features need a release note. -->

```release-note
None: Exclude this PR from the release note.
```



Co-authored-by: zhangsoledad <787953403@qq.com>
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