-
Notifications
You must be signed in to change notification settings - Fork 12
feat: use fvm precompiles for native payments, burn, beacon randomness #207
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
Conversation
This branch also has #206 because the fvm-solidity library currently requires 0.8.30. |
src/PDPVerifier.sol
Outdated
|
||
// Decode and return the result | ||
return abi.decode(result, (uint256)); | ||
return epoch.getBeaconRandomness(); |
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'm not a huge fan of augmenting uint256 like this. It makes the source of this less clear. It feels a touch too clever.
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.
src/PDPVerifier.sol
Outdated
function burnFee(uint256 amount) internal { | ||
require(msg.value >= amount, "Incorrect fee amount"); | ||
(bool success,) = BURN_ACTOR.call{value: amount}(""); | ||
bool success = amount.burn(); |
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.
Same thing as with randomness, IM,O there is no harm in it being FVMPay.burn(amount)
and it makes the code much more readable.
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.
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.
SGTM, just got some conflicts
Reviewer @rvagg Similar to FilOzone/pdp#207 and FilOzone/filecoin-pay#234 #### Changes * burn with fvm pay * mock with MockFVMTest
Reviewer @ZenGround0
https://github.com/filecoin-project/fvm-solidity
I made this library to reuse the precompile code and its mocks across repositories, after demonstrating that the precompile native send method uses less gas.
See also the related filecoin-pay changeset: FilOzone/filecoin-pay#234
Changes