Skip to content

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

Merged
merged 10 commits into from
Apr 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

106 changes: 84 additions & 22 deletions target_chains/ethereum/contracts/contracts/entropy/Entropy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ 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";
import "@pythnetwork/entropy-sdk-solidity/EntropyStatusConstants.sol";

// Entropy implements a secure 2-party random number generation procedure. The protocol
// is an extension of a simple commit/reveal protocol. The original version has the following steps:
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -247,7 +251,9 @@ abstract contract Entropy is IEntropy, EntropyState {

req.blockNumber = SafeCast.toUint64(block.number);
req.useBlockhash = useBlockhash;
req.isRequestWithCallback = isRequestWithCallback;
req.callbackStatus = isRequestWithCallback
? EntropyStatusConstants.CALLBACK_NOT_STARTED
: EntropyStatusConstants.CALLBACK_NOT_NECESSARY;
}

// As a user, request a random number from `provider`. Prior to calling this method, the user should
Expand Down Expand Up @@ -403,7 +409,7 @@ abstract contract Entropy is IEntropy, EntropyState {
}

// Fulfill a request for a random number. This method validates the provided userRandomness and provider's proof
// against the corresponding commitments in the in-flight request. If both values are validated, this function returns
// against the corresponding commitments in the in-flight request. If both values are validated, this method returns
// the corresponding random number.
//
// Note that this function can only be called once per in-flight request. Calling this function deletes the stored
Expand All @@ -423,7 +429,9 @@ abstract contract Entropy is IEntropy, EntropyState {
sequenceNumber
);

if (req.isRequestWithCallback) {
if (
req.callbackStatus != EntropyStatusConstants.CALLBACK_NOT_NECESSARY
) {
revert EntropyErrors.InvalidRevealCall();
}

Expand Down Expand Up @@ -467,9 +475,14 @@ abstract contract Entropy is IEntropy, EntropyState {
sequenceNumber
);

if (!req.isRequestWithCallback) {
if (
!(req.callbackStatus ==
EntropyStatusConstants.CALLBACK_NOT_STARTED ||
req.callbackStatus == EntropyStatusConstants.CALLBACK_FAILED)
) {
revert EntropyErrors.InvalidRevealCall();
}

bytes32 blockHash;
bytes32 randomNumber;
(randomNumber, blockHash) = revealHelper(
Expand All @@ -480,26 +493,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.callbackStatus == EntropyStatusConstants.CALLBACK_NOT_STARTED) {
req.callbackStatus = EntropyStatusConstants.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.
Copy link
Collaborator

Choose a reason for hiding this comment

The 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.callbackStatus = EntropyStatusConstants.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.callbackStatus = EntropyStatusConstants.CALLBACK_FAILED;
} else {
// The callback ran out of gas
Copy link
Collaborator

Choose a reason for hiding this comment

The 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
);
}
}
}

Expand Down
Loading
Loading