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

Fix the preimage display at height=0 #187

Merged
merged 1 commit into from
Nov 16, 2020
Merged

Fix the preimage display at height=0 #187

merged 1 commit into from
Nov 16, 2020

Conversation

TrustHenry
Copy link
Member

@TrustHenry TrustHenry commented Nov 11, 2020 •
edited
Loading

At height=0, the distance = 0 and the preimage is displayed as randomHash

The enrollments of the Genesis block starts with zero avail_height.

Fixed #167

Geod24
Geod24 reviewed Nov 11, 2020
View reviewed changes
src/Stoa.ts Outdated
Comment on lines 176 to 195
// Genesis block height
if (target_height == new Height(0n))
{
preimage_distance = 0;
result_preimage_hash = row.random_seed;
}
else
{
preimage_distance = NaN;
result_preimage_hash = Hash.NULL;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it special-casing the genesis block ? Any enrollment contains the preimage at distance 0.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right. I'm testing it now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed it

TrustHenry
TrustHenry commented Nov 11, 2020
View reviewed changes
src/Stoa.ts Show resolved Hide resolved
@AndrejMitrovic
Copy link

Why is the code for getValidator and getValidators copy pasted?

@TrustHenry
Copy link
Member Author

Why is the code for getValidator and getValidators copy pasted?

I didn't want to use the cloned code either,
but I haven't moved the business code yet.
I'll refactoring.

@AndrejMitrovic
Copy link

Alright, it would be good to refactor it.

@TrustHenry
Copy link
Member Author

Alright, it would be good to refactor it.

Related to #189

MichaelKim20
MichaelKim20 reviewed Nov 13, 2020
View reviewed changes
src/Stoa.ts Outdated Show resolved Hide resolved
@MichaelKim20
Copy link
Member

Is it normal that there is no data when API 'GET /validators/' is called with a height value 0?
The enrollment data for the genesis block has an enrolled_at of 0.

@TrustHenry
Copy link
Member Author

Is it normal that there is no data when API 'GET /validators/' is called with a height value 0?
The enrollment data for the genesis block has an enrolled_at of 0.

I just applied the comment above, so it should work.

@TrustHenry TrustHenry force-pushed the distance branch 3 times, most recently from b753dbc to 2f059ef Compare November 13, 2020 06:42
@TrustHenry
Copy link
Member Author

It works well.

@TrustHenry TrustHenry force-pushed the distance branch 3 times, most recently from 2d0d0c0 to c9f64aa Compare November 13, 2020 09:26
MichaelKim20
MichaelKim20 reviewed Nov 16, 2020
View reviewed changes
tests/Utils.ts Outdated Show resolved Hide resolved
MichaelKim20
MichaelKim20 reviewed Nov 16, 2020
View reviewed changes
tests/Utils.ts Outdated Show resolved Hide resolved
@TrustHenry
Fix the preimage display at height=0
3dab8ce 
The enrollmentis of the Genesis block starts with zero avail_height.
MichaelKim20
MichaelKim20 approved these changes Nov 16, 2020
View reviewed changes
Copy link
Member

@MichaelKim20 MichaelKim20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked that the pre-image becomes random-seed when the distance is zero in each cycle.
LGTM

@MichaelKim20 MichaelKim20 merged commit da4730d into bosagora:v0.x.x Nov 16, 2020
@TrustHenry TrustHenry deleted the distance branch February 4, 2021 05:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Geod24 Geod24 Geod24 left review comments

@AndrejMitrovic AndrejMitrovic AndrejMitrovic left review comments

@MichaelKim20 MichaelKim20 MichaelKim20 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.

Off by one error in pre-image handling
4 participants
@TrustHenry @AndrejMitrovic @MichaelKim20 @Geod24