Create MockEntropy.sol #1745
Create MockEntropy.sol #1745hanbao-dev wants to merge 4 commits intopyth-network:mainfrom hanbao-dev:main
Conversation
|
@danielstevenberger is attempting to deploy a commit to the pyth-web Team on Vercel. A member of the Team first needs to authorize it. |
| bytes32 randomNumberBytes = bytes32(_randomNumber); | ||
| IEntropyConsumer(callbackAddress)._entropyCallback( | ||
| _sequenceNumber, | ||
| callbackAddress, |
|
|
||
| function triggerCallback( | ||
| uint256 _randomNumber, | ||
| uint64 _sequenceNumber |
There was a problem hiding this comment.
why is there a leading _ here?
| uint64 _sequenceNumber | ||
| ) external { | ||
| address callbackAddress = callbackRequests[_sequenceNumber]; | ||
| require(callbackAddress != address(0), "No pending request"); |
There was a problem hiding this comment.
use the same errors we have in EntropyErrors.sol. Here it should be NoSuchRequest
There was a problem hiding this comment.
Updated to use EntropyErrors.sol
| address _provider, | ||
| bytes32 _userRandomNumber | ||
| ) external payable isProvider(_provider) returns (uint64) { | ||
| require(msg.value >= fee, "Not enough ether sent for fee"); |
There was a problem hiding this comment.
use the same errors we have in EntropyErrors.sol. Here it should be InsufficientFee
There was a problem hiding this comment.
Updated to use EntropyErrors.sol
| } | ||
|
|
||
| modifier isProvider(address _provider) { | ||
| require(_provider == address(this), "Invalid provider address"); |
There was a problem hiding this comment.
use the same errors we have in EntropyErrors.sol. Here it should be NoSuchProvider
There was a problem hiding this comment.
Updated to use EntropyErrors.sol
Removed unnecessary "_" in inputs Utilized EntropyErrors.sol errors
| import {IEntropyConsumer} from "./IEntropyConsumer.sol"; | ||
| import "./EntropyErrors.sol"; | ||
|
|
||
| contract MockEntropy { |
There was a problem hiding this comment.
I think this should inherit from IEntropy and define all of the methods there, so it's a drop-in replacement for testing.
There was a problem hiding this comment.
(it's ok to throw an error saying "not implemented" for methods like register which are targeted toward providers though)
jayantk
left a comment
jayantk
left a comment
There was a problem hiding this comment.
I think this approach generally makes sense. I have a couple asks in the comments here:
- Please implement all parts of the IEntropy interface that an end user could reasonably invoke. You can ignore the methods that only providers will call, but there are other getters that should be implemented on the mock
- Please write a unit test using the mock
- please document how to use the mock in a doc comment on the contract + a method comment on the triggerCallback method
| } | ||
|
|
||
| function getDefaultProvider() external view returns (address) { | ||
| return address(this); |
There was a problem hiding this comment.
I suggest requiring the caller to pass a provider address as a constructor argument. Using the address of the deployed contract could cause mixups in the tested code (e.g., it won't catch a bug where someone passes the Entropy contract address as the provider address by mistake).
| import {IEntropyConsumer} from "./IEntropyConsumer.sol"; | ||
| import "./EntropyErrors.sol"; | ||
|
|
||
| contract MockEntropy { |
There was a problem hiding this comment.
(it's ok to throw an error saying "not implemented" for methods like register which are targeted toward providers though)
| import {IEntropyConsumer} from "./IEntropyConsumer.sol"; | ||
| import "./EntropyErrors.sol"; | ||
|
|
||
| contract MockEntropy { |
There was a problem hiding this comment.
please document this mock contract and how to use it
Please also add a unit test in https://github.com/pyth-network/pyth-crosschain/tree/main/target_chains/ethereum/contracts/forge-test to validate that this mock behaves as expected
|
@jayantk please first confirm this is what you're looking for. I've done testing in my local with hardhat for price and triggering the callback that I can add here using forge if this approach aligns. The functions below are all not implemented
The default provider get's registered in the constructor |
3 participants
Add MockEntropy Contract for Local Testing
Description:
This PR adds the MockEntropy contract to facilitate local testing and development. The contract includes the following functionalities:
Key Feature:
triggerCallback enables flexible and customizable callback handling for testing purposes.