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

RFC31: Add a variable length field in the block header #224

Merged
merged 6 commits into from
Apr 1, 2022
Merged

RFC31: Add a variable length field in the block header #224

merged 6 commits into from
Apr 1, 2022

Conversation

doitian
Copy link
Member

@doitian doitian commented Feb 7, 2021

This document proposes adding an optional variable length field to the block header.

@doitian doitian changed the title variable-length-header-field Add a variable length field in the block header Feb 7, 2021
@doitian doitian added b:consensus Break consensus hard-fork labels Feb 7, 2021
@doitian doitian mentioned this pull request Apr 25, 2021
Closed
10 tasks
@doitian doitian added the ckb2021 Hard fork scheduled in 2021 label Apr 26, 2021
@doitian doitian marked this pull request as ready for review May 17, 2021 05:05
@doitian doitian requested a review from a team as a code owner May 17, 2021 05:05
quake
quake previously approved these changes May 18, 2021
View reviewed changes
@yangby-cryptape yangby-cryptape mentioned this pull request May 31, 2021
Merged
bors bot added a commit to nervosnetwork/ckb that referenced this pull request Jun 15, 2021
@bors @zhangsoledad @yangby-cryptape
Merge #2715
b0ab80e 
2715: feat(hardfork): ckb2021 hardfork features r=yangby-cryptape a=yangby-cryptape

### Suggestions for Review

**The best way to review this PR is reviewing each commit independently.**

### Brief introduction for Commits

#### New hardfork Features

**All following features can be found in RFC PRs.**

- feat(hardfork): in the "since epoch", the index should be less than length

  Ref: [CKB-RFCs PR 223: Ensure that index < length in input since field using epoch](nervosnetwork/rfcs#223)

- feat(hardfork): use block timestamp of input cells as relative since start timestamp

  Ref: [CKB-RFCs PR 221: Use Block Timestamp as Start Timestamp in Since](nervosnetwork/rfcs#221)

- **[Reverted]** ~~feat(hardfork): allow unknown block versions and transactions versions~~

  ~~Ref: [CKB-RFCs PR 230: Allow unknown tx & block version](nervosnetwork/rfcs#230

- feat(hardfork): allow script multiple matches on identical data for type hash-type scripts

  Ref: [CKB-RFCs PR 222: Allow script multiple matches on identical code](nervosnetwork/rfcs#222)

- feat(hardfork): reuse the uncles hash in the header as the extra hash

  Ref: [CKB-RFCs PR 224: Add a variable length field in the block header](nervosnetwork/rfcs#224)

- **[Reverted]** ~~feat(hardfork): allow loading uncommitted cell data hashes from tx pool~~

  ~~Ref: [CKB-RFCs PR 228: ckb2021: fix load_cell_data_hash syscall](nervosnetwork/rfcs#228

#### Other Important Commits

- feat(hardfork): setup the components for hard fork features

- refactor: let verifiers know the real environment that the transaction is in

  Almost all features require this refactor commit.

- refactor: remove useless parameter "with_data" because it always be true (tricky)

  So I can change less APIs and less code to apply the feature: allow loading uncommitted cell data hashes from tx pool.

### About Tests

Almost all features have detailed integration tests (or unit tests):
- Many blocks before hardfork;
- Only one block before hardfork;
- The block at hardfork;
- Many blocks after hardfork.

All commits can passed all integration tests and unit tests.

Co-authored-by: zhangsoledad <[email protected]>
Co-authored-by: Boyu Yang <[email protected]>
bors bot added a commit to nervosnetwork/ckb that referenced this pull request Jun 15, 2021
@bors @zhangsoledad @yangby-cryptape
Merge #2715
85d04c3 
2715: feat(hardfork): ckb2021 hardfork features r=quake,doitian,zhangsoledad a=yangby-cryptape

### Suggestions for Review

**The best way to review this PR is reviewing each commit independently.**

### Brief introduction for Commits

#### New hardfork Features

**All following features can be found in RFC PRs.**

- feat(hardfork): in the "since epoch", the index should be less than length

  Ref: [CKB-RFCs PR 223: Ensure that index < length in input since field using epoch](nervosnetwork/rfcs#223)

- feat(hardfork): use block timestamp of input cells as relative since start timestamp

  Ref: [CKB-RFCs PR 221: Use Block Timestamp as Start Timestamp in Since](nervosnetwork/rfcs#221)

- **[Reverted]** ~~feat(hardfork): allow unknown block versions and transactions versions~~

  ~~Ref: [CKB-RFCs PR 230: Allow unknown tx & block version](nervosnetwork/rfcs#230

- feat(hardfork): allow script multiple matches on identical data for type hash-type scripts

  Ref: [CKB-RFCs PR 222: Allow script multiple matches on identical code](nervosnetwork/rfcs#222)

- feat(hardfork): reuse the uncles hash in the header as the extra hash

  Ref: [CKB-RFCs PR 224: Add a variable length field in the block header](nervosnetwork/rfcs#224)

- **[Reverted]** ~~feat(hardfork): allow loading uncommitted cell data hashes from tx pool~~

  ~~Ref: [CKB-RFCs PR 228: ckb2021: fix load_cell_data_hash syscall](nervosnetwork/rfcs#228

#### Other Important Commits

- feat(hardfork): setup the components for hard fork features

- refactor: let verifiers know the real environment that the transaction is in

  Almost all features require this refactor commit.

- refactor: remove useless parameter "with_data" because it always be true (tricky)

  So I can change less APIs and less code to apply the feature: allow loading uncommitted cell data hashes from tx pool.

### About Tests

Almost all features have detailed integration tests (or unit tests):
- Many blocks before hardfork;
- Only one block before hardfork;
- The block at hardfork;
- Many blocks after hardfork.

All commits can passed all integration tests and unit tests.

Co-authored-by: zhangsoledad <[email protected]>
Co-authored-by: Boyu Yang <[email protected]>
@doitian doitian changed the title Add a variable length field in the block header RFC224: Add a variable length field in the block header Jul 22, 2021
@doitian doitian force-pushed the variable-length-header-field branch from 17bcb5f to 26b80ca Compare July 22, 2021 15:28
@doitian doitian mentioned this pull request Jul 23, 2021
Open
14 tasks
@doitian doitian changed the title RFC224: Add a variable length field in the block header RFC31: Add a variable length field in the block header Jul 23, 2021
@doitian doitian force-pushed the variable-length-header-field branch from 26b80ca to 18b45d3 Compare July 23, 2021 07:36
@yangby-cryptape
Copy link
Contributor

In RFC 0020:

The total size of the first four fields should be no larger than the hard-coded block size limit. The main purpose of implementing a block size limit is to avoid overloading public nodes' bandwidth. The uncle blocks’ proposal zones do not count in the limit as they are usually already synchronized when the block is mined.

Whether the extension field count in the block size limit or not?

@yangby-cryptape
Copy link
Contributor

The extension field is not in UncleBlock, I think we should mention that in this RFC.

@doitian
Copy link
Member Author

doitian commented Aug 11, 2021

In RFC 0020:

The total size of the first four fields should be no larger than the hard-coded block size limit. The main purpose of implementing a block size limit is to avoid overloading public nodes' bandwidth. The uncle blocks’ proposal zones do not count in the limit as they are usually already synchronized when the block is mined.

Whether the extension field count in the block size limit or not?

Included in the block size limit.

Keith-CY added a commit to ckb-js/ckb-sdk-js that referenced this pull request Aug 30, 2021
@Keith-CY
feat(rpc): a new field 'extension' is added to the block body
285a829 
A new field 'extension' is added to the block body

BREAKING CHANGE: A new field 'extension' is added to the block body

ref nervosnetwork/rfcs#224
@Keith-CY Keith-CY mentioned this pull request Aug 30, 2021
Merged
@Keith-CY Keith-CY mentioned this pull request Oct 22, 2021
Merged
yangby-cryptape
yangby-cryptape previously approved these changes Jan 4, 2022
View reviewed changes
@doitian doitian requested review from janx and quake March 2, 2022 03:49
@doitian doitian requested review from xxuejie and removed request for quake, knwang and zhangsoledad March 4, 2022 02:12
@doitian
Copy link
Member Author

doitian commented Mar 4, 2022

Assigned reviewers: @xxuejie @janx

janx
janx previously approved these changes Mar 4, 2022
View reviewed changes
@doitian doitian mentioned this pull request Mar 14, 2022
Open
@doitian doitian requested a review from quake March 17, 2022 02:25
@doitian doitian added the s:fcp Final Comment Period. PR is merged when there's no comments in 14 days. label Mar 17, 2022
@doitian
Copy link
Member Author

doitian commented Mar 18, 2022

FCP will end on 2022-04-01

@doitian doitian merged commit 57c129c into nervosnetwork:master Apr 1, 2022
@doitian doitian deleted the variable-length-header-field branch April 1, 2022 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xxuejie xxuejie xxuejie approved these changes

@janx janx janx approved these changes

@yangby-cryptape yangby-cryptape yangby-cryptape left review comments

@quake quake Awaiting requested review from quake

Assignees
No one assigned
Labels
b:consensus Break consensus ckb2021 Hard fork scheduled in 2021 hard-fork s:fcp Final Comment Period. PR is merged when there's no comments in 14 days.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@doitian @yangby-cryptape @janx @quake @xxuejie