-
Notifications
You must be signed in to change notification settings - Fork 80
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
feat:implement beneficiary fip0029 #496
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks great so far. Could you please port the tests too?
@anorth could give a test example, i dont know how to write test in this project |
See https://github.com/filecoin-project/builtin-actors/blob/master/actors/miner/tests/change_owner_address_test.rs and other files in that dir |
why this pr's ci not running? |
It's possibly related to it being from a fork. @ZenGround0 @aarshkshah1992 do we have experience with this from other contributions? |
af42b15
to
2f56193
Compare
Thanks, I will review again now the tests are there |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the tests, great work.
Some minor comments, and a request for one new test to demonstrate what happens when the owner and beneficiary attempt to reduce the quota.
3687586
to
e501e28
Compare
fce9db4
to
db0e8b7
Compare
Unfortunately I haven't given a thorough review yet and I'll be offline for a week but the high level looks good and the testing looks great. If necessary I will do a review after the fact so don't block on me if you need to merge before then. |
16c2f21
to
8cd5400
Compare
This is an implementation for FIP-0029 PR filecoin-project/specs-actors#1571 to provide Beneficiary address support. In this implementation, version and upgrading is not considered, that is, directly put the code change into the current version. The back-compliance could be done once the next version content is determined.
Closes #422