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

RFC28: Use Block Timestamp as Start Timestamp in Since #221

Merged
merged 5 commits into from
Jan 21, 2022
Merged

RFC28: Use Block Timestamp as Start Timestamp in Since #221

merged 5 commits into from
Jan 21, 2022

Conversation

doitian
Copy link
Member

@doitian doitian commented Feb 3, 2021

No description provided.

@doitian doitian requested a review from a team as a code owner February 3, 2021 13:01
@doitian doitian added b:consensus Break consensus hard-fork labels Feb 4, 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
@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 <787953403@qq.com>
Co-authored-by: Boyu Yang <yangby@cryptape.com>
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 <787953403@qq.com>
Co-authored-by: Boyu Yang <yangby@cryptape.com>
@doitian
Copy link
Member Author

doitian commented Jun 26, 2021

@keroro520 fixed the inequality a1f3442

@doitian doitian requested a review from keroro520 June 26, 2021 04:57
keroro520
keroro520 reviewed Jun 26, 2021
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.

LGTM

@doitian doitian changed the title RFC: Use Block Timestamp as Start Timestamp in Since RFC221: Use Block Timestamp as Start Timestamp in Since Jul 22, 2021
@doitian doitian force-pushed the change-since-relative-timestamp-consensus branch from a1f3442 to 31175d8 Compare July 22, 2021 14:21
@doitian doitian mentioned this pull request Jul 23, 2021
Open
14 tasks
@doitian doitian changed the title RFC221: Use Block Timestamp as Start Timestamp in Since RFC28: Use Block Timestamp as Start Timestamp in Since Jul 23, 2021
@doitian doitian force-pushed the change-since-relative-timestamp-consensus branch from 31175d8 to 0a70dd4 Compare July 23, 2021 07:22
@doitian
RFC28: Use Block Timestamp as Start Timestamp in Since
efd2d36 
This document proposes a transaction verification consensus change. When the `since` field of the transaction input uses a relative timestamp, the referenced cell committing block timestamp is used instead of the median timestamp.

This is a modification to the RFC17 Transaction valid since
@doitian doitian force-pushed the change-since-relative-timestamp-consensus branch from 0a70dd4 to efd2d36 Compare July 23, 2021 07:32
@keroro520 keroro520 mentioned this pull request Aug 10, 2021
Merged
keroro520
keroro520 previously approved these changes Nov 12, 2021
View reviewed changes
yangby-cryptape
yangby-cryptape previously approved these changes Jan 4, 2022
View reviewed changes
@doitian @janx
Apply suggestions from code review
50a73bd 
Co-authored-by: busyforking <5958+janx@users.noreply.github.com>
@doitian doitian requested a review from janx January 14, 2022 01:37
@doitian doitian requested a review from janx January 21, 2022 05:20
@janx janx merged commit b118094 into nervosnetwork:master Jan 21, 2022
@doitian doitian deleted the change-since-relative-timestamp-consensus branch February 23, 2023 06:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yangby-cryptape yangby-cryptape yangby-cryptape approved these changes

@quake quake quake approved these changes

@janx janx janx approved these changes

@keroro520 keroro520 keroro520 left review comments

@knwang knwang Awaiting requested review from knwang

@xxuejie xxuejie Awaiting requested review from xxuejie

Assignees
No one assigned
Labels
b:consensus Break consensus ckb2021 Hard fork scheduled in 2021 soft-fork
Projects
None yet
Milestone
No milestone
Development

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