-
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: FVM no longer draws randomness for you #1378
Conversation
runtime/src/runtime/fvm.rs
Outdated
match e { | ||
ErrorNumber::LimitExceeded => { | ||
actor_error!(illegal_argument; "randomness lookback exceeded: {}", e) | ||
} | ||
e => actor_error!(assertion_failed; "get chain randomness failed with an unexpected error: {}", e), | ||
} | ||
}) | ||
})?; | ||
Ok(draw_randomness(&digest, personalization, rand_epoch, entropy)) |
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.
I was initially intending to do this in the SDK but... yeah, it actually makes sense to do it here given that the FVM doesn't really care about how the user draws randomness anyways.
So LGTM?
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.
Yeah, this seems like the better thing to do? It does mean that all contracts need to implement their own "drawing" logic, but that's a good thing, arguably.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## asr/next #1378 +/- ##
===========================================
Coverage ? 90.76%
===========================================
Files ? 145
Lines ? 27286
Branches ? 0
===========================================
Hits ? 24766
Misses ? 2520
Partials ? 0 |
@@ -19,3 +21,63 @@ pub enum DomainSeparationTag { | |||
PoStChainCommit = 9, | |||
EvmPrevRandao = 10, | |||
} | |||
|
|||
pub fn draw_randomness( |
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.
Clippy is convinced this is unused, because it is only called from the FVM-specific code. Is there a more sophisticated way to fix this than slapping allow(unused) on it?
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.
Not really.
@@ -19,3 +21,63 @@ pub enum DomainSeparationTag { | |||
PoStChainCommit = 9, | |||
EvmPrevRandao = 10, | |||
} | |||
|
|||
pub fn draw_randomness( |
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.
Not really.
You'll need to rebase as well. |
6a76d41
to
ce83ed7
Compare
ce83ed7
to
8ddec2f
Compare
8ddec2f
to
dc11008
Compare
Integrates filecoin-project/ref-fvm#1848 from the FVM
next
workstream.Test to come.