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

FIP 0008 Batch Pre Commits #1189

Merged
merged 8 commits into from
Aug 19, 2021
Merged

FIP 0008 Batch Pre Commits #1189

merged 8 commits into from
Aug 19, 2021

Conversation

ec2
Copy link
Member

@ec2 ec2 commented Jul 12, 2021

Summary of changes
Changes introduced in this pull request:

Reference issue to close (if applicable)

Closes #1106

Other information and links

@ec2 ec2 requested a review from a user July 12, 2021 01:49
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

LGTM, although I'd like to run this on a branch that synchronizes the state tree with the rest of the Network Version #13 changes.

Since NV13 is already live, is there anywhere I can look to see how these changes impact network congestion or miner costs?

@cryptoquick
Copy link
Contributor

Agreed, this would be easier to review if it wasn't in separate PRs, imo. What branch syncs for you, @ec2 ?

@ec2
Copy link
Member Author

ec2 commented Jul 22, 2021

@olibearo @cryptoquick You will be able to sync with ec2/v5

@ec2 ec2 linked an issue Aug 4, 2021 that may be closed by this pull request
Copy link
Contributor

@cryptoquick cryptoquick left a comment

Choose a reason for hiding this comment

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

Reviewed against changes in PRs linked here:
#1106 (comment)

Needs a few changes, some minor, some might be actual oversights.

vm/actor/src/builtin/miner/mod.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/state.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/state.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/state.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@cryptoquick cryptoquick left a comment

Choose a reason for hiding this comment

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

Alright, this one looks good.

@ec2 ec2 merged commit ea98ea2 into main Aug 19, 2021
@ec2 ec2 deleted the ec2/fip8 branch August 19, 2021 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement FIP 0008 Batched PreCommits
2 participants