Skip to content

Commit 1c27dda

Browse files
jayantkjayantk
committed
tests
1 parent 330b8ee commit 1c27dda

File tree

5 files changed

+191
-49
lines changed

5 files changed

+191
-49
lines changed

‎target_chains/ethereum/contracts/contracts/entropy/Entropy.sol‎

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ abstract contract Entropy is IEntropy, EntropyState {
103103
for (uint8 i = 0; i < NUM_REQUESTS; i++) {
104104
EntropyStructs.Request storage req = _state.requests[i];
105105
req.provider = address(1);
106-
req.blockNumber = 1234;
106+
req.blockNumberOrGasLimit = 1234;
107107
req.commitment = hex"0123";
108108
}
109109
}
@@ -222,14 +222,12 @@ abstract contract Entropy is IEntropy, EntropyState {
222222
providerInfo.sequenceNumber += 1;
223223

224224
// Check that fees were paid and increment the pyth / provider balances.
225-
uint128 requiredFee = getFee(provider);
225+
uint128 requiredFee = getFeeForGas(provider, callbackGasLimit);
226226
if (msg.value < requiredFee) revert EntropyErrors.InsufficientFee();
227-
providerInfo.accruedFeesInWei += getProviderFee(
228-
provider,
229-
callbackGasLimit
230-
);
227+
uint128 providerFee = getProviderFee(provider, callbackGasLimit);
228+
providerInfo.accruedFeesInWei += providerFee;
231229
_state.accruedPythFeesInWei += (SafeCast.toUint128(msg.value) -
232-
providerInfo.feeInWei);
230+
providerFee);
233231

234232
// Store the user's commitment so that we can fulfill the request later.
235233
// Warning: this code needs to overwrite *every* field in the request, because the returned request can be
@@ -257,16 +255,19 @@ abstract contract Entropy is IEntropy, EntropyState {
257255
} else if (isRequestWithCallback) {
258256
req.isRequestWithCallback = isRequestWithCallback;
259257
if (callbackGasLimit == 0) {
260-
req.blockNumber = providerInfo.defaultGasLimit;
258+
req.blockNumberOrGasLimit = providerInfo.defaultGasLimit;
261259
} else {
262-
req.blockNumber = callbackGasLimit;
260+
req.blockNumberOrGasLimit = callbackGasLimit;
263261
}
264262
req.useBlockhash = false;
265263
} else {
266264
req.isRequestWithCallback = false;
267-
req.blockNumber = SafeCast.toUint64(block.number);
265+
req.blockNumberOrGasLimit = SafeCast.toUint64(block.number);
268266
req.useBlockhash = useBlockhash;
269267
}
268+
269+
req.callbackFailed = false;
270+
req.reentryGuard = false;
270271
}
271272

272273
// As a user, request a random number from `provider`. Prior to calling this method, the user should
@@ -376,9 +377,9 @@ abstract contract Entropy is IEntropy, EntropyState {
376377

377378
blockHash = bytes32(uint256(0));
378379
if (req.useBlockhash) {
379-
bytes32 _blockHash = blockhash(req.blockNumber);
380+
bytes32 _blockHash = blockhash(req.blockNumberOrGasLimit);
380381

381-
// The `blockhash` function will return zero if the req.blockNumber is equal to the current
382+
// The `blockhash` function will return zero if the req.blockNumberOrGasLimit is equal to the current
382383
// block number, or if it is not within the 256 most recent blocks. This allows the user to
383384
// select between two random numbers by executing the reveal function in the same block as the
384385
// request, or after 256 blocks. This gives each user two chances to get a favorable result on
@@ -451,7 +452,7 @@ abstract contract Entropy is IEntropy, EntropyState {
451452
}
452453

453454
// Fulfill a request for a random number. This method validates the provided userRandomness and provider's proof
454-
// against the corresponding commitments in the in-flight request. If both values are validated, this function returns
455+
// against the corresponding commitments in the in-flight request. If both values are validated, this method returns
455456
// the corresponding random number.
456457
//
457458
// Note that this function can only be called once per in-flight request. Calling this function deletes the stored
@@ -520,7 +521,7 @@ abstract contract Entropy is IEntropy, EntropyState {
520521
}
521522
// Invariant check: all callback requests should have useBlockhash set to false.
522523
if (req.useBlockhash) {
523-
revert EntropyErrors.InvalidRevealCall();
524+
revert EntropyErrors.AssertionFailure();
524525
}
525526

526527
if (req.reentryGuard) {
@@ -537,11 +538,16 @@ abstract contract Entropy is IEntropy, EntropyState {
537538

538539
address callAddress = req.requester;
539540

540-
if (req.blockNumber != 0 && !req.callbackAttempted) {
541+
// blockNumberOrGasLimit holds the gas limit in the callback case.
542+
// If the gas limit is 0, then the provider hasn't configured their default limit,
543+
// so we default to the prior entropy flow (where there is no failure state).
544+
// Similarly, if the request has already failed, we fall back to the prior flow so that
545+
// recovery attempts can provide more gas / directly see the revert reason.
546+
if (req.blockNumberOrGasLimit != 0 && !req.callbackFailed) {
541547
// TODO: need to validate that we have enough gas left to forward (?)
542548
// Or at least that we forwarded enough gas before marking the callback as failed
543549
/*
544-
if (gasleft() < req.blockNumber) {
550+
if (gasleft() < req.blockNumberOrGasLimit) {
545551
546552
}
547553
*/
@@ -550,7 +556,7 @@ abstract contract Entropy is IEntropy, EntropyState {
550556
bool success;
551557
bytes memory ret;
552558
(success, ret) = callAddress.excessivelySafeCall(
553-
req.blockNumber,
559+
req.blockNumberOrGasLimit,
554560
0,
555561
32,
556562
abi.encodeWithSelector(
@@ -582,7 +588,7 @@ abstract contract Entropy is IEntropy, EntropyState {
582588
sequenceNumber,
583589
errorReason
584590
);
585-
req.callbackAttempted = true;
591+
req.callbackFailed = true;
586592
}
587593
} else {
588594
emit RevealedWithCallback(
@@ -600,11 +606,13 @@ abstract contract Entropy is IEntropy, EntropyState {
600606
}
601607

602608
if (len != 0) {
609+
req.reentryGuard = true;
603610
IEntropyConsumer(callAddress)._entropyCallback(
604611
sequenceNumber,
605612
provider,
606613
randomNumber
607614
);
615+
req.reentryGuard = false;
608616
}
609617
}
610618
}
@@ -652,6 +660,8 @@ abstract contract Entropy is IEntropy, EntropyState {
652660
providerAddr
653661
];
654662
if (gasLimit > provider.defaultGasLimit) {
663+
// This calculation rounds down the fee, which means that users can get some gas in the callback for free.
664+
// However, the value of the free gas is < 1 wei, which is insignificant.
655665
uint128 additionalFee = ((gasLimit - provider.defaultGasLimit) *
656666
provider.feeInWei) / provider.defaultGasLimit;
657667
return provider.feeInWei + additionalFee;

‎target_chains/ethereum/contracts/forge-test/Entropy.t.sol‎

Lines changed: 137 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,10 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents {
203203
function testBasicFlow() public {
204204
vm.roll(17);
205205
uint64 sequenceNumber = request(user2, provider1, 42, false);
206-
assertEq(random.getRequest(provider1, sequenceNumber).blockNumber, 17);
206+
assertEq(
207+
random.getRequest(provider1, sequenceNumber).blockNumberOrGasLimit,
208+
17
209+
);
207210
assertEq(
208211
random.getRequest(provider1, sequenceNumber).useBlockhash,
209212
false
@@ -243,7 +246,10 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents {
243246
42,
244247
false
245248
);
246-
assertEq(random.getRequest(provider1, sequenceNumber).blockNumber, 20);
249+
assertEq(
250+
random.getRequest(provider1, sequenceNumber).blockNumberOrGasLimit,
251+
20
252+
);
247253
assertEq(
248254
random.getRequest(provider1, sequenceNumber).useBlockhash,
249255
false
@@ -405,7 +411,7 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents {
405411
uint64 sequenceNumber = request(user2, provider1, 42, true);
406412

407413
assertEq(
408-
random.getRequest(provider1, sequenceNumber).blockNumber,
414+
random.getRequest(provider1, sequenceNumber).blockNumberOrGasLimit,
409415
1234
410416
);
411417
assertEq(
@@ -801,11 +807,11 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents {
801807
providerInfo.currentCommitment
802808
)
803809
),
804-
blockNumber: 0,
810+
blockNumberOrGasLimit: 0,
805811
requester: user1,
806812
useBlockhash: false,
807813
isRequestWithCallback: true,
808-
callbackAttempted: false,
814+
callbackFailed: false,
809815
reentryGuard: false
810816
})
811817
);
@@ -1150,6 +1156,132 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents {
11501156
vm.expectRevert();
11511157
random.setProviderFeeAsFeeManager(provider1, 1000);
11521158
}
1159+
1160+
function testSetDefaultGasLimit() public {
1161+
uint64 newGasLimit = 100000;
1162+
1163+
vm.prank(provider1);
1164+
random.setDefaultGasLimit(newGasLimit);
1165+
1166+
EntropyStructs.ProviderInfo memory info = random.getProviderInfo(
1167+
provider1
1168+
);
1169+
assertEq(info.defaultGasLimit, newGasLimit);
1170+
}
1171+
1172+
function testSetDefaultGasLimitRevertIfNotFromProvider() public {
1173+
vm.expectRevert(EntropyErrors.NoSuchProvider.selector);
1174+
random.setDefaultGasLimit(100000);
1175+
}
1176+
1177+
function testRequestWithCallbackUsesDefaultGasLimit() public {
1178+
uint64 defaultGasLimit = 100000;
1179+
vm.prank(provider1);
1180+
random.setDefaultGasLimit(defaultGasLimit);
1181+
1182+
bytes32 userRandomNumber = bytes32(uint(42));
1183+
uint fee = random.getFee(provider1);
1184+
1185+
vm.deal(user1, fee);
1186+
vm.prank(user1);
1187+
uint64 sequenceNumber = random.requestWithCallback{value: fee}(
1188+
provider1,
1189+
userRandomNumber
1190+
);
1191+
1192+
EntropyStructs.Request memory req = random.getRequest(
1193+
provider1,
1194+
sequenceNumber
1195+
);
1196+
assertEq(req.blockNumberOrGasLimit, defaultGasLimit);
1197+
}
1198+
1199+
function testRequestWithCallbackAndCustomGasLimit() public {
1200+
uint64 defaultGasLimit = 100000;
1201+
uint64 customGasLimit = 200000;
1202+
1203+
vm.prank(provider1);
1204+
random.setDefaultGasLimit(defaultGasLimit);
1205+
1206+
bytes32 userRandomNumber = bytes32(uint(42));
1207+
uint fee = random.getFeeForGas(provider1, customGasLimit);
1208+
1209+
vm.deal(user1, fee);
1210+
vm.prank(user1);
1211+
uint64 sequenceNumber = random.requestWithCallbackAndGas{value: fee}(
1212+
provider1,
1213+
userRandomNumber,
1214+
customGasLimit
1215+
);
1216+
1217+
EntropyStructs.Request memory req = random.getRequest(
1218+
provider1,
1219+
sequenceNumber
1220+
);
1221+
assertEq(req.blockNumberOrGasLimit, customGasLimit);
1222+
}
1223+
1224+
function testRequestWithCallbackAndGasLimitFeeScaling() public {
1225+
uint64 defaultGasLimit = 100000;
1226+
uint64 doubleGasLimit = 200000;
1227+
1228+
vm.prank(provider1);
1229+
random.setDefaultGasLimit(defaultGasLimit);
1230+
1231+
uint baseFee = random.getFee(provider1);
1232+
assertEq(baseFee, provider1FeeInWei + pythFeeInWei);
1233+
1234+
// Fee scales proportionally with gas limit
1235+
uint scaledFee = random.getFeeForGas(provider1, doubleGasLimit);
1236+
assertEq(scaledFee, 2 * provider1FeeInWei + pythFeeInWei);
1237+
}
1238+
1239+
function testRequestWithCallbackAndGasLimitInsufficientFee() public {
1240+
uint64 defaultGasLimit = 100000;
1241+
uint64 doubleGasLimit = 200000;
1242+
1243+
vm.prank(provider1);
1244+
random.setDefaultGasLimit(defaultGasLimit);
1245+
1246+
bytes32 userRandomNumber = bytes32(uint(42));
1247+
uint baseFee = random.getFee(provider1); // This is insufficient for double gas
1248+
1249+
vm.deal(user1, baseFee);
1250+
vm.prank(user1);
1251+
vm.expectRevert(EntropyErrors.InsufficientFee.selector);
1252+
random.requestWithCallbackAndGas{value: baseFee}(
1253+
provider1,
1254+
userRandomNumber,
1255+
doubleGasLimit
1256+
);
1257+
}
1258+
1259+
function testRequestWithCallbackAndGasLimitLowerThanDefault() public {
1260+
uint64 defaultGasLimit = 100000;
1261+
uint64 lowerGasLimit = 50000;
1262+
1263+
vm.prank(provider1);
1264+
random.setDefaultGasLimit(defaultGasLimit);
1265+
1266+
bytes32 userRandomNumber = bytes32(uint(42));
1267+
uint fee = random.getFeeForGas(provider1, lowerGasLimit);
1268+
1269+
vm.deal(user1, fee);
1270+
vm.prank(user1);
1271+
uint64 sequenceNumber = random.requestWithCallbackAndGas{value: fee}(
1272+
provider1,
1273+
userRandomNumber,
1274+
lowerGasLimit
1275+
);
1276+
1277+
EntropyStructs.Request memory req = random.getRequest(
1278+
provider1,
1279+
sequenceNumber
1280+
);
1281+
assertEq(req.blockNumberOrGasLimit, lowerGasLimit);
1282+
// Fee should be the same as base fee since we're using less gas than default
1283+
assertEq(fee, random.getFee(provider1));
1284+
}
11531285
}
11541286

11551287
contract EntropyConsumer is IEntropyConsumer {

‎target_chains/ethereum/entropy_sdk/solidity/EntropyStructs.sol‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,22 +52,22 @@ contract EntropyStructs {
5252
// eliminating 1 store.
5353
bytes32 commitment;
5454
// Storage slot 3 //
55-
// The number of the block where this request was created.
56-
// Note that we're using a uint64 such that we have an additional space for an address and other fields in
55+
// The number of the block where this request was created OR the gas limit for callbacks.
56+
// (isRequestedWithCallback toggles between these two cases. Block numbers are not required
57+
// in the callback case, as these are used to determine blockhashes, which aren't used.)
58+
//
59+
// Note that we're using a uint64 for the blockNumber such that we have an additional space for an address and other fields in
5760
// this storage slot. Although block.number returns a uint256, 64 bits should be plenty to index all of the
5861
// blocks ever generated.
59-
//
60-
// Note: We are overloading this storage slot to also store a gas limit for callbacks, as we do not support
61-
// blockhashes in the callback case.
62-
uint64 blockNumber;
62+
uint64 blockNumberOrGasLimit;
6363
// The address that requested this random number.
6464
address requester;
6565
// If true, incorporate the blockhash of blockNumber into the generated random value.
6666
bool useBlockhash;
6767
// If true, the requester will be called back with the generated random value.
6868
bool isRequestWithCallback;
69-
// If true, the callback has been attempted by the provider (and failed for some reason).
70-
bool callbackAttempted;
69+
// If true, the callback has been attempted by the provider and failed for some reason.
70+
bool callbackFailed;
7171
// If true, a fulfillment request for this is already in-flight
7272
bool reentryGuard;
7373
}

0 commit comments

Comments
 (0)