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.

Sign up for GitHub

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

Jump to bottom

refactor: Add CellDep and change OutPoint data structure #1356

Merged
merged 16 commits into from
Aug 11, 2019
Merged

refactor: Add CellDep and change OutPoint data structure #1356

merged 16 commits into from
Aug 11, 2019

Conversation

TheWaWaR
Copy link
Contributor

@TheWaWaR TheWaWaR commented Aug 8, 2019 •
edited
Loading

  1. Add a new CellDep data structure, and changed Transaction:
// For support dep group
pub struct CellDep {
    out_point: OutPoint,
    is_dep_group: bool,
}

// deps changed to cell_deps and header_deps
pub struct Transaction {
    // ...
    cell_deps: Vec<CellDep>,
    header_deps: Vec<H256>,
    // ...
}

  1. OutPoint data structure renamed from origin CellOutPoint.

  2. HeaderProvider changed to HeaderChecker because header now lazy loaded.

  3. SourceEntry rename Dep to CellDep and add a new variant CellHeader, you can load CellHeader from deps.headers by index in VM.

enum SourceEntry {
    Input,
    Output,
    // Cell dep
    CellDep,
    // Header dep
    CellHeader,
}

@TheWaWaR TheWaWaR requested a review from a team August 8, 2019 08:23
@nervos-bot
Copy link

nervos-bot bot commented Aug 8, 2019

@zhangsoledad is assigned as the chief reviewer

@TheWaWaR TheWaWaR force-pushed the refactor-core-cell-dep branch from a994829 to 777c071 Compare August 8, 2019 08:58
@TheWaWaR TheWaWaR mentioned this pull request Aug 8, 2019
Closed
7 tasks
@TheWaWaR TheWaWaR force-pushed the refactor-core-cell-dep branch from 777c071 to 1c1fde2 Compare August 8, 2019 11:57
@doitian doitian mentioned this pull request Aug 8, 2019
Closed
@driftluo driftluo self-requested a review August 9, 2019 06:24
@doitian
Copy link
Member

doitian commented Aug 9, 2019 •
edited
Loading

The Dep in SourceEntry should rename to CellDep and Header should be named HeaderDep

keroro520
keroro520 reviewed Aug 9, 2019
View reviewed changes
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.

The Dep in SourceEntry should rename to CellDep and Header should be named HeaderDep

What about DepCell and DepHeader?

@TheWaWaR
Copy link
Contributor Author

TheWaWaR commented Aug 9, 2019

The Dep in SourceEntry should rename to CellDep and Header should be named HeaderDep

What about DepCell and DepHeader?

Would be inconsistent with cell_deps and header_deps.

@keroro520
Copy link
Contributor

Would be inconsistent with cell_deps and header_deps.

So ...... Why not rename to dep_cells and dep_headers?

@TheWaWaR
Copy link
Contributor Author

TheWaWaR commented Aug 9, 2019

So ...... Why not rename to dep_cells and dep_headers?

Because they are dependencies, and we just talked about it.

@TheWaWaR TheWaWaR force-pushed the refactor-core-cell-dep branch from 96fd217 to fbbf2cc Compare August 9, 2019 11:15
u2
u2 reviewed Aug 10, 2019
View reviewed changes
core/src/transaction.rs Show resolved Hide resolved
indexer/src/types.rs Show resolved Hide resolved
protocol/src/protocol.fbs Show resolved Hide resolved
protocol/src/protocol.fbs Show resolved Hide resolved
u2
u2 suggested changes Aug 10, 2019
View reviewed changes
script/src/syscalls/load_header.rs Show resolved Hide resolved
script/src/syscalls/load_header.rs Show resolved Hide resolved
u2
u2 reviewed Aug 10, 2019
View reviewed changes
shared/src/cell_set.rs Show resolved Hide resolved
shared/src/cell_set.rs Show resolved Hide resolved
shared/src/chain_state.rs Show resolved Hide resolved
core/src/transaction.rs Show resolved Hide resolved
@u2 u2 self-requested a review August 10, 2019 05:07
@u2 u2 requested review from keroro520 and u2 August 10, 2019 05:08
@TheWaWaR TheWaWaR force-pushed the refactor-core-cell-dep branch from fbbf2cc to 937d793 Compare August 10, 2019 18:52
@TheWaWaR
refactor: Add CellDep and change OutPoint data structure
ed9c2c8 
BREAKING CHANGE: core data structure CellDep/OutPoint/CellInput/Transaction changed
@TheWaWaR
refactor(jsonrpc-types): Update for OutPoint/CellDep
ff8837c 
BREAKING CHANGE: Transaction/OutPoint data structure changed
@TheWaWaR
refactor(protocol): Update for OutPoint/CellDep
2613a87 
BREAKING CHANGE: Protocol schema changed
@TheWaWaR
refactor(protos): Update for OutPoint/CellDep
a50680b 
BREAKING CHANGE: schema changed
@TheWaWaR
refactor(store): Update for core changes
675f6eb
@TheWaWaR TheWaWaR force-pushed the refactor-core-cell-dep branch from 937d793 to 7a6fb0b Compare August 10, 2019 19:17
@doitian
Copy link
Member

doitian commented Aug 11, 2019

The PR description is out dated.

@TheWaWaR
Copy link
Contributor Author

The PR description is out dated.

Updated

@TheWaWaR
Copy link
Contributor Author

bors r+

@nervos-bot nervos-bot bot added the s:ready-to-merge Status: Waiting to be merged. label Aug 11, 2019
bors bot added a commit that referenced this pull request Aug 11, 2019
@bors @TheWaWaR
Merge #1356
21cf079 
1356: refactor: Add CellDep and change OutPoint data structure r=TheWaWaR a=TheWaWaR

1. Add a new `CellDep` data structure, and changed `Transaction`:
``` rust
// For support dep group
pub struct CellDep {
    out_point: OutPoint,
    is_dep_group: bool,
}

// deps changed to cell_deps and header_deps
pub struct Transaction {
    // ...
    cell_deps: Vec<CellDep>,
    header_deps: Vec<H256>,
    // ...
}
```

2. `OutPoint` data structure renamed from origin `CellOutPoint`.
3. `HeaderProvider` changed to `HeaderChecker` because header now lazy loaded.

4. `SourceEntry` rename `Dep` to `CellDep` and add a new variant `CellHeader`, you can load `CellHeader` from `deps.headers` by index in VM.
``` rust
enum SourceEntry {
    Input,
    Output,
    // Cell dep
    CellDep,
    // Header dep
    CellHeader,
}
```

Co-authored-by: Linfeng Qian <[email protected]>
Co-authored-by: Qian Linfeng <[email protected]>
@bors
Copy link
Contributor

bors bot commented Aug 11, 2019

Build failed

  • continuous-integration/travis-ci/push

@TheWaWaR TheWaWaR merged commit 7bb0d61 into nervosnetwork:develop Aug 11, 2019
@doitian doitian added this to the v0.18.0 and older milestone Aug 23, 2019
@doitian doitian mentioned this pull request Aug 24, 2019
Merged
@TheWaWaR TheWaWaR deleted the refactor-core-cell-dep branch August 27, 2019 05:50
@keroro520 keroro520 mentioned this pull request Sep 7, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@quake quake quake approved these changes

@driftluo driftluo Awaiting requested review from driftluo

@keroro520 keroro520 Awaiting requested review from keroro520

@u2 u2 Awaiting requested review from u2

Assignees

@zhangsoledad zhangsoledad

Labels
s:ready-to-merge Status: Waiting to be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@TheWaWaR @doitian @keroro520 @quake @u2 @driftluo @zhangsoledad