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: resolve conflict descendants #3264

Conversation

zhangsoledad
Copy link
Member

@zhangsoledad zhangsoledad commented Jan 1, 2022

What problem does this PR solve?

Previously, pending transaction removed but descendants remain.

What is changed and how it works?

  1. remove conflict and descendants.
  2. transaction expiration.
  3. non-mine mode.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code ci-runs-only: [ quick_checks,linters ]

Release note

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

@zhangsoledad zhangsoledad requested a review from a team as a code owner January 1, 2022 13:28
@zhangsoledad zhangsoledad force-pushed the zhangsoledad/resolve_conflict_descendants branch from dc47d53 to c25306c Compare January 1, 2022 15:12
tx-pool/src/process.rs Show resolved Hide resolved
tx-pool/src/process.rs Show resolved Hide resolved
tx-pool/src/process.rs Outdated Show resolved Hide resolved
tx-pool/src/process.rs Outdated Show resolved Hide resolved
tx-pool/src/component/pending.rs Outdated Show resolved Hide resolved
tx-pool/src/component/pending.rs Show resolved Hide resolved
tx-pool/src/component/pending.rs Outdated Show resolved Hide resolved
tx-pool/src/process.rs Outdated Show resolved Hide resolved
@zhangsoledad zhangsoledad force-pushed the zhangsoledad/resolve_conflict_descendants branch from 641ec4a to f382e8a Compare January 4, 2022 10:07
@chanhsu001 chanhsu001 requested review from yangby-cryptape and removed request for chanhsu001 January 6, 2022 01:00
@quake
Copy link
Member

quake commented Jan 6, 2022

bors r=quake,yangby-cryptape

@bors bors bot merged commit d9a407c into nervosnetwork:develop Jan 6, 2022
@zhangsoledad zhangsoledad deleted the zhangsoledad/resolve_conflict_descendants branch January 6, 2022 12:32
@zhangsoledad zhangsoledad mentioned this pull request Jan 9, 2022
bors bot added a commit that referenced this pull request Jan 10, 2022
3270: fix pending leak r=quake,yangby-cryptape 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 is changed and how it works?

fix memory leak introduce from #3264

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

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

- Unit test
previous will get failures:
    component::tests::pending::test_basic
    component::tests::pending::test_fill_proposals
    component::tests::pending::test_resolve_conflict_header_dep
```
failures:

---- component::tests::pending::test_basic stdout ----
thread 'component::tests::pending::test_basic' panicked at 'assertion failed: `(left == right)`
  left: `8`,
 right: `4`', tx-pool/src/component/tests/pending.rs:26:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- component::tests::pending::test_fill_proposals stdout ----
thread 'component::tests::pending::test_fill_proposals' panicked at 'assertion failed: `(left == right)`
  left: `13`,
 right: `7`', tx-pool/src/component/tests/pending.rs:204:5

---- component::tests::pending::test_resolve_conflict_header_dep stdout ----
thread 'component::tests::pending::test_resolve_conflict_header_dep' panicked at 'assertion failed: `(left == right)`
  left: `4`,
 right: `2`', tx-pool/src/component/tests/pending.rs:121:5
```



### 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.
Title Only: Include only the PR title in the release note.
Note: Add a note under the PR title in the release note.
```



Co-authored-by: zhangsoledad <[email protected]>
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