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.

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

Cuckatoo size shift upgrade schedule #2024

Merged
merged 7 commits into from
Nov 28, 2018

Conversation

ignopeverell
Copy link
Contributor

Fixes #2001

@ignopeverell ignopeverell requested a review from tromp November 26, 2018 17:54
Copy link
Contributor

@tromp tromp left a comment

Choose a reason for hiding this comment

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

31 + (height / YEAR_HEIGHT)
is an exponential size increase; not the linear one we want.
what we want is 31 + log_2(height / YEAR_HEIGHT)

but instead I propose to leave DEFAULT_MIN_EDGE_BITS the constant it was,
and rely on the graph weight function to codify the phasing out schedule,
since we need that anyway.

@ignopeverell
Copy link
Contributor Author

@tromp just pushed an update that moves the schedule into graph_weight. It could be written more succinctly or elegantly but I wanted to first have an agreement on that method before finicking.

Copy link
Contributor

@tromp tromp left a comment

Choose a reason for hiding this comment

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

I find it clearer in term of an explicitly computed expiry height, e.g.

    let bits_over_min = edge_bits - global::min_edge_bits();
    let expiry_height = (1 << bits_over_min) * YEAR_HEIGHT;
    if height > expiry_height {
            xpr_edge_bits =
                    xpr_edge_bits.saturating_sub(1 + (height - expiry_height) / WEEK_HEIGHT);
    }

@ignopeverell
Copy link
Contributor Author

It is clearer. Finished up the PR and have the test passing now. Should be ready for merge.

@ignopeverell ignopeverell merged commit 28e0d97 into mimblewimble:master Nov 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants