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-0095: First draft of add FEVM precompile to fetch beacon digest from chain history #1048

Merged
merged 11 commits into from
Sep 3, 2024

Conversation

ZenGround0
Copy link
Contributor

@ZenGround0 ZenGround0 commented Aug 26, 2024

The FVM exposes a syscall for fetching the digest of a drand beacon included in some prior Filecoin chain header.
The built-in actors use this facility to provide randomness for storage proofs.
This proposal introduces an EVM actor precompile exposing this functionality to user-deployed smart contracts
so that they can also use random challenges for other types of proofs.

Discussion: #1051

@ZenGround0 ZenGround0 changed the title Rand EVM precompile get_randomness EVM precompile Aug 26, 2024
@Stebalien
Copy link
Member

Could you add a discussion topic?

@anorth anorth changed the title get_randomness EVM precompile First draft of add EVM precompile to fetch beacon digest from chain history Aug 27, 2024
@anorth anorth marked this pull request as draft August 27, 2024 01:35
@anorth
Copy link
Member

anorth commented Aug 27, 2024

@jennijuju thanks for being so speedy, but I don't think this PR was ready for review yet. It should probably have been marked draft while @ZenGround0 and I collaborate on the details.

FIPS/fip-rand-precompile.md Outdated Show resolved Hide resolved
|------------------|---------------------------|
| randomness_epoch | U256 - low i64 |

The call errors with `PrecompileError::InvalidInput` if the epoch is invalid.
Copy link
Member

Choose a reason for hiding this comment

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

should outline what "invalid" means here, just future epochs? negative i64's?

Copy link
Member

Choose a reason for hiding this comment

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

I'v adjusted it slightly, but I think it's easier to define this via what positively is valid than the inverse.

Copy link
Contributor Author

@ZenGround0 ZenGround0 left a comment

Choose a reason for hiding this comment

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

This looks good to me and ready to turn into a real PR

@anorth anorth marked this pull request as ready for review August 27, 2024 21:22
Copy link
Contributor

@lanzafame lanzafame left a comment

Choose a reason for hiding this comment

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

missing full stop.

FIPS/fip-rand-precompile.md Outdated Show resolved Hide resolved
The call errors with `PrecompileError::InvalidInput` if the epoch is not a valid, prior epoch number.

This calls directly to the FVMs `get_beacon_randomness` syscall, which returns a Blake2b hash digest of the
randomness value from the drand beacon at the associated Filecoin epoch.
Copy link
Member

Choose a reason for hiding this comment

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

what would happen if it was a null round of a given filecoin epoch?

Copy link
Member

Choose a reason for hiding this comment

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

Whatever would already happen in that case, which this FIP doesn't propose to change.

I can see a reasonable motivation to want to describe this for potential users, which now extend beyond built-in actors. This FIP isn't proposing to change any of the semantics, but they are poorly documented at present. I will try to come back with some edits to describe them here as context, but I need some help because I wasn't involved in developing the underlying syscall.

@jennijuju jennijuju changed the title First draft of add EVM precompile to fetch beacon digest from chain history FIP-0095: First draft of add EVM precompile to fetch beacon digest from chain history Aug 29, 2024
@anorth anorth changed the title FIP-0095: First draft of add EVM precompile to fetch beacon digest from chain history FIP-0095: First draft of add FEVM precompile to fetch beacon digest from chain history Aug 29, 2024
@anorth
Copy link
Member

anorth commented Aug 29, 2024

I've pushed edits in response to review.

I think this draft is ready for merge. I acknowledge the outstanding request for more detail about semantics of the underlying syscall, but since this FIP is not proposing to change any of them, I don't think that should block progress. I'll endeavour to come back with an update to describe that better in a subsequent PR.

@jennijuju
Copy link
Member

I've pushed edits in response to review.

I think this draft is ready for merge. I acknowledge the outstanding request for more detail about semantics of the underlying syscall, but since this FIP is not proposing to change any of them, I don't think that should block progress. I'll endeavour to come back with an update to describe that better in a subsequent PR.

#1051 (comment)

Copy link
Contributor

@momack2 momack2 left a comment

Choose a reason for hiding this comment

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

Nice to add another value-add FEVM feature!

@jennijuju jennijuju assigned jennijuju and unassigned jennijuju Sep 3, 2024
@jennijuju jennijuju merged commit 83d2a92 into master Sep 3, 2024
1 check passed
@jennijuju jennijuju deleted the fip-xxxx/evm-randomness-precompile branch September 3, 2024 21:01
but is missing the hook-up to where user-deployed smart contracts can use it.
This proposal provides that hook-up.

The EVM does already provide a PREVRANDAO opcode, but this is limited to access randomness from the immediately prior epoch.
Copy link
Contributor

@CluEleSsUK CluEleSsUK Sep 3, 2024

Choose a reason for hiding this comment

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

I'm very late to the party here, but I don't understand the motivation of having an opcode for searching the block history vs just exposing a method for verifying drand/arbitrary BLS signatures.
If the caller is triggering some PDP by calling the user space smart contract, providing the relevant drand signature as calldata seems much more flexible (and could allow for granularity better than filecoin block time).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@CluEleSsUK yes BLS precompiles are the generally better solution. We essentially introduced this FIP to work around the PDP timeline.

Basically this FIP has the two properties of

  1. it's is an overall improvement and a nice positive change for the network
  2. we could get a FIP ready for last call to land in nv24 and ship PDP this year

We would love to see BLS precompiles and @anorth started exploring this direction before flagging it would take too much time. If your team would like to continue work on BLS precompiles we can share what we've learned so far and support you getting it into nv25.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One other nice thing about this FIP is the potentially efficiency speedup. Since the network has gone through the trouble to validate these signatures once it is convenient for users' gas margins to make use of the relatively cheaper fetching of the already validated randomness.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think there’s still appetite from our team to pick up the BLS precompiles but I can’t commit to a timeline just yet. Will discuss it next week

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.

9 participants