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

[Merged by Bors] - Remove timestamp #2343

Closed
wants to merge 4 commits into from
Closed

[Merged by Bors] - Remove timestamp #2343

wants to merge 4 commits into from

Conversation

noamnelke
Copy link
Member

Motivation

Closes #2225 (obsoletes #2231)

Changes

Remove Timestamp field from the BlockHeader struct.

Test Plan

Unit tests have been updated and all pass consistently. System tests should be unaffected and will be run.

DevOps Notes

  • Does this code require configuration changes? ❌
  • Does this code affect public APIs? ❌
  • Does this code assume a new version of external services (PoET, elasticsearch, etc.) ❌
  • Does this code make changes to log messages that our monitoring infrastructure may rely on? ❌

krishnaindani and others added 2 commits March 22, 2021 09:15
@krishnaindani @noamnelke
remove timestamp from block header
fe61cb8
@noamnelke
TestBlockBuilder_BlockIdGeneration
250ebfa 
This test depended on the timestamp field to makes blocks different. This is not the correct behavior and now that the timestamp was removed blocks only differed by the coin value (some of the time). This fixes the test by using different ATXs for each block, which makes them consistently different.
@noamnelke
Copy link
Member Author

bors try

bors bot added a commit that referenced this pull request Mar 22, 2021
@bors
Try #2343:
31d7719
@bors
Copy link

bors bot commented Mar 22, 2021

try

Build failed:

@noamnelke
Copy link
Member Author

@antonlerner weird behavior from Github actions - it reported ci-stage1 as both skipped and passing and didn't report ci-stage2 at all. All test passed, though - so ignore the "failed" bors try.

@lrettig
Copy link
Member

lrettig commented Mar 22, 2021

bors try

bors bot added a commit that referenced this pull request Mar 22, 2021
@bors
Try #2343:
350fbec
@bors
Copy link

bors bot commented Mar 22, 2021

try

Build failed:

@lrettig
Copy link
Member

lrettig commented Mar 22, 2021

bors try

bors bot added a commit that referenced this pull request Mar 22, 2021
@bors
Try #2343:
5579f22
@bors
Copy link

bors bot commented Mar 22, 2021

try

Build succeeded:

@noamnelke
Copy link
Member Author

bors merge

bors bot pushed a commit that referenced this pull request Mar 25, 2021
@krishnaindani @noamnelke @lrettig
Remove timestamp (#2343)
573e0bb 
## Motivation
Closes #2225 (obsoletes #2231)

## Changes
Remove `Timestamp` field from the `BlockHeader` struct.

## Test Plan
Unit tests have been updated and all pass consistently. System tests should be unaffected and will be run.

## DevOps Notes
- [x] Does this code require configuration changes? ❌ 
- [x] Does this code affect public APIs? ❌ 
- [x] Does this code assume a new version of external services (PoET, elasticsearch, etc.) ❌ 
- [x] Does this code make changes to log messages that our monitoring infrastructure may rely on? ❌ 

Co-authored-by: Noam Nelke <noamnelke@gmail.com>
Co-authored-by: Lane Rettig <lanerettig@gmail.com>
@bors
Copy link

bors bot commented Mar 25, 2021

Pull request successfully merged into develop.

Build succeeded:

@bors bors bot changed the title Remove timestamp [Merged by Bors] - Remove timestamp Mar 25, 2021
@bors bors bot closed this Mar 25, 2021
@bors bors bot deleted the remove_timestamp branch March 25, 2021 11:44
lrettig added a commit that referenced this pull request Mar 30, 2021
@krishnaindani @noamnelke @lrettig
Remove timestamp (#2343)
aa43f3a 
## Motivation
Closes #2225 (obsoletes #2231)

## Changes
Remove `Timestamp` field from the `BlockHeader` struct.

## Test Plan
Unit tests have been updated and all pass consistently. System tests should be unaffected and will be run.

## DevOps Notes
- [x] Does this code require configuration changes? ❌ 
- [x] Does this code affect public APIs? ❌ 
- [x] Does this code assume a new version of external services (PoET, elasticsearch, etc.) ❌ 
- [x] Does this code make changes to log messages that our monitoring infrastructure may rely on? ❌ 

Co-authored-by: Noam Nelke <noamnelke@gmail.com>
Co-authored-by: Lane Rettig <lanerettig@gmail.com>
lrettig added a commit that referenced this pull request Mar 30, 2021
@krishnaindani @noamnelke @lrettig
Remove timestamp (#2343)
aeae1b1 
## Motivation
Closes #2225 (obsoletes #2231)

## Changes
Remove `Timestamp` field from the `BlockHeader` struct.

## Test Plan
Unit tests have been updated and all pass consistently. System tests should be unaffected and will be run.

## DevOps Notes
- [x] Does this code require configuration changes? ❌ 
- [x] Does this code affect public APIs? ❌ 
- [x] Does this code assume a new version of external services (PoET, elasticsearch, etc.) ❌ 
- [x] Does this code make changes to log messages that our monitoring infrastructure may rely on? ❌ 

Co-authored-by: Noam Nelke <noamnelke@gmail.com>
Co-authored-by: Lane Rettig <lanerettig@gmail.com>
lrettig added a commit that referenced this pull request Mar 30, 2021
@krishnaindani @noamnelke @lrettig
Remove timestamp (#2343)
5ba46b1 
## Motivation
Closes #2225 (obsoletes #2231)

## Changes
Remove `Timestamp` field from the `BlockHeader` struct.

## Test Plan
Unit tests have been updated and all pass consistently. System tests should be unaffected and will be run.

## DevOps Notes
- [x] Does this code require configuration changes? ❌ 
- [x] Does this code affect public APIs? ❌ 
- [x] Does this code assume a new version of external services (PoET, elasticsearch, etc.) ❌ 
- [x] Does this code make changes to log messages that our monitoring infrastructure may rely on? ❌ 

Co-authored-by: Noam Nelke <noamnelke@gmail.com>
Co-authored-by: Lane Rettig <lanerettig@gmail.com>
lrettig added a commit that referenced this pull request Mar 31, 2021
@krishnaindani @noamnelke @lrettig
Remove timestamp (#2343)
c2c087c 
## Motivation
Closes #2225 (obsoletes #2231)

## Changes
Remove `Timestamp` field from the `BlockHeader` struct.

## Test Plan
Unit tests have been updated and all pass consistently. System tests should be unaffected and will be run.

## DevOps Notes
- [x] Does this code require configuration changes? ❌ 
- [x] Does this code affect public APIs? ❌ 
- [x] Does this code assume a new version of external services (PoET, elasticsearch, etc.) ❌ 
- [x] Does this code make changes to log messages that our monitoring infrastructure may rely on? ❌ 

Co-authored-by: Noam Nelke <noamnelke@gmail.com>
Co-authored-by: Lane Rettig <lanerettig@gmail.com>
lrettig added a commit that referenced this pull request Apr 1, 2021
@krishnaindani @noamnelke @lrettig
Remove timestamp (#2343)
8975537 
## Motivation
Closes #2225 (obsoletes #2231)

## Changes
Remove `Timestamp` field from the `BlockHeader` struct.

## Test Plan
Unit tests have been updated and all pass consistently. System tests should be unaffected and will be run.

## DevOps Notes
- [x] Does this code require configuration changes? ❌ 
- [x] Does this code affect public APIs? ❌ 
- [x] Does this code assume a new version of external services (PoET, elasticsearch, etc.) ❌ 
- [x] Does this code make changes to log messages that our monitoring infrastructure may rely on? ❌ 

Co-authored-by: Noam Nelke <noamnelke@gmail.com>
Co-authored-by: Lane Rettig <lanerettig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@antonlerner antonlerner antonlerner approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove timestamp field from block header
4 participants
@noamnelke @lrettig @antonlerner @krishnaindani