-
Notifications
You must be signed in to change notification settings - Fork 292
feat(entropy): Add callback failure states to the contract #2546
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
Changes from 5 commits
99b3ad1
717d701
bac4cce
63733b3
491a01e
24e5a55
e7bd6b7
f99040d
c7703b1
4d2aaca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,11 +3,13 @@ | |
pragma solidity ^0.8.0; | ||
|
||
import "@pythnetwork/entropy-sdk-solidity/EntropyStructs.sol"; | ||
import "@pythnetwork/entropy-sdk-solidity/EntropyConstants.sol"; | ||
import "@pythnetwork/entropy-sdk-solidity/EntropyErrors.sol"; | ||
import "@pythnetwork/entropy-sdk-solidity/EntropyEvents.sol"; | ||
import "@pythnetwork/entropy-sdk-solidity/IEntropy.sol"; | ||
import "@pythnetwork/entropy-sdk-solidity/IEntropyConsumer.sol"; | ||
import "@openzeppelin/contracts/utils/math/SafeCast.sol"; | ||
import "@nomad-xyz/excessively-safe-call/src/ExcessivelySafeCall.sol"; | ||
import "./EntropyState.sol"; | ||
|
||
// Entropy implements a secure 2-party random number generation procedure. The protocol | ||
|
@@ -76,6 +78,8 @@ import "./EntropyState.sol"; | |
// the user is always incentivized to reveal their random number, and that the protocol has an escape hatch for | ||
// cases where the user chooses not to reveal. | ||
abstract contract Entropy is IEntropy, EntropyState { | ||
using ExcessivelySafeCall for address; | ||
|
||
function _initialize( | ||
address admin, | ||
uint128 pythFeeInWei, | ||
|
@@ -247,7 +251,9 @@ abstract contract Entropy is IEntropy, EntropyState { | |
|
||
req.blockNumber = SafeCast.toUint64(block.number); | ||
req.useBlockhash = useBlockhash; | ||
req.isRequestWithCallback = isRequestWithCallback; | ||
req.status = isRequestWithCallback | ||
? EntropyConstants.STATUS_CALLBACK_NOT_STARTED | ||
: EntropyConstants.STATUS_NO_CALLBACK; | ||
} | ||
|
||
// As a user, request a random number from `provider`. Prior to calling this method, the user should | ||
|
@@ -423,7 +429,7 @@ abstract contract Entropy is IEntropy, EntropyState { | |
sequenceNumber | ||
); | ||
|
||
if (req.isRequestWithCallback) { | ||
if (req.status != EntropyConstants.STATUS_NO_CALLBACK) { | ||
revert EntropyErrors.InvalidRevealCall(); | ||
} | ||
|
||
|
@@ -467,9 +473,13 @@ abstract contract Entropy is IEntropy, EntropyState { | |
sequenceNumber | ||
); | ||
|
||
if (!req.isRequestWithCallback) { | ||
if ( | ||
!(req.status == EntropyConstants.STATUS_CALLBACK_NOT_STARTED || | ||
req.status == EntropyConstants.STATUS_CALLBACK_FAILED) | ||
) { | ||
revert EntropyErrors.InvalidRevealCall(); | ||
} | ||
|
||
bytes32 blockHash; | ||
bytes32 randomNumber; | ||
(randomNumber, blockHash) = revealHelper( | ||
|
@@ -480,26 +490,75 @@ abstract contract Entropy is IEntropy, EntropyState { | |
|
||
address callAddress = req.requester; | ||
|
||
emit RevealedWithCallback( | ||
req, | ||
userRandomNumber, | ||
providerRevelation, | ||
randomNumber | ||
); | ||
|
||
clearRequest(provider, sequenceNumber); | ||
|
||
// Check if the callAddress is a contract account. | ||
uint len; | ||
assembly { | ||
len := extcodesize(callAddress) | ||
} | ||
if (len != 0) { | ||
IEntropyConsumer(callAddress)._entropyCallback( | ||
sequenceNumber, | ||
provider, | ||
// Requests that haven't been invoked yet will be invoked safely (catching reverts), and | ||
// any reverts will be reported as an event. Any failing requests move to a failure state | ||
// at which point they can be recovered. The recovery flow invokes the callback directly | ||
// (no catching errors) which allows callers to easily see the revert reason. | ||
if (req.status == EntropyConstants.STATUS_CALLBACK_NOT_STARTED) { | ||
req.status = EntropyConstants.STATUS_CALLBACK_IN_PROGRESS; | ||
bool success; | ||
bytes memory ret; | ||
(success, ret) = callAddress.excessivelySafeCall( | ||
gasleft(), // TODO: providers need to be able to configure this in the future. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this actually needs to be exactly what the requester asked for and not what provider specifyes |
||
256, // copy at most 256 bytes of the return value into ret. | ||
abi.encodeWithSelector( | ||
IEntropyConsumer._entropyCallback.selector, | ||
sequenceNumber, | ||
provider, | ||
randomNumber | ||
) | ||
); | ||
// Reset status to not started here in case the transaction reverts. | ||
req.status = EntropyConstants.STATUS_CALLBACK_NOT_STARTED; | ||
|
||
if (success) { | ||
emit RevealedWithCallback( | ||
req, | ||
userRandomNumber, | ||
providerRevelation, | ||
randomNumber | ||
); | ||
clearRequest(provider, sequenceNumber); | ||
} else if (ret.length > 0) { | ||
// Callback reverted for some reason that is *not* out-of-gas. | ||
emit CallbackFailed( | ||
provider, | ||
req.requester, | ||
sequenceNumber, | ||
userRandomNumber, | ||
providerRevelation, | ||
randomNumber, | ||
ret | ||
); | ||
req.status = EntropyConstants.STATUS_CALLBACK_FAILED; | ||
} else { | ||
// The callback ran out of gas | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there are other failure scenarios where ret.length is 0. |
||
// TODO: this case will go away once we add provider gas limits, so we're not putting in a custom error type. | ||
require(false, "provider needs to send more gas"); | ||
} | ||
} else { | ||
// This case uses the checks-effects-interactions pattern to avoid reentry attacks | ||
emit RevealedWithCallback( | ||
req, | ||
userRandomNumber, | ||
providerRevelation, | ||
randomNumber | ||
); | ||
|
||
clearRequest(provider, sequenceNumber); | ||
|
||
// Check if the callAddress is a contract account. | ||
uint len; | ||
assembly { | ||
len := extcodesize(callAddress) | ||
} | ||
if (len != 0) { | ||
IEntropyConsumer(callAddress)._entropyCallback( | ||
sequenceNumber, | ||
provider, | ||
randomNumber | ||
); | ||
} | ||
} | ||
} | ||
|
||
|
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.
Note that this is a behavior change at the moment. This means that we won't be able to upgrade the contracts without upgrading the keeper.
However, in the next PR I will additionally condition this on the request having a gas limit, which means that the contract will be 100% backward compatible. (i.e., you don't get the failure flow until the provider explicitly sets a gas limit for callbacks)