-
Notifications
You must be signed in to change notification settings - Fork 301
feat(pulse): add pulse contracts #2090
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 all commits
911887f
d05d869
f017f29
19ae7ea
fbbae70
061dfb2
ccaab98
27f665b
035938f
a7a4d23
183658e
e957c79
54d310e
2ca191a
c455937
b313728
8f64744
bce0770
9d5cd7e
81e4535
cda5a2b
604b113
dedca30
d748162
5399eb0
7cd8328
9e046dc
1f82d5e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,3 +23,4 @@ __pycache__ | |
.direnv | ||
.next | ||
.turbo/ | ||
.cursorrules |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
// SPDX-License-Identifier: Apache 2 | ||
|
||
pragma solidity ^0.8.0; | ||
|
||
import "@pythnetwork/pyth-sdk-solidity/IPyth.sol"; | ||
import "./PulseEvents.sol"; | ||
import "./PulseState.sol"; | ||
|
||
interface IPulseConsumer { | ||
function pulseCallback( | ||
uint64 sequenceNumber, | ||
address updater, | ||
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 we should have |
||
PythStructs.PriceFeed[] memory priceFeeds | ||
) external; | ||
} | ||
|
||
interface IPulse is PulseEvents { | ||
// Core functions | ||
/** | ||
* @notice Requests price updates with a callback | ||
* @dev The msg.value must be equal to getFee(callbackGasLimit) | ||
* @param callbackGasLimit The amount of gas allocated for the callback execution | ||
* @param publishTime The minimum publish time for price updates, it should be less than or equal to block.timestamp + 60 | ||
* @param priceIds The price feed IDs to update. Maximum 10 price feeds per request. | ||
* Requests requiring more feeds should be split into multiple calls. | ||
* @return sequenceNumber The sequence number assigned to this request | ||
* @dev Security note: The 60-second future limit on publishTime prevents a DoS vector where | ||
* attackers could submit many low-fee requests for far-future updates when gas prices | ||
* are low, forcing executors to fulfill them later when gas prices might be much higher. | ||
* Since tx.gasprice is used to calculate fees, allowing far-future requests would make | ||
* the fee estimation unreliable. | ||
*/ | ||
function requestPriceUpdatesWithCallback( | ||
uint256 publishTime, | ||
bytes32[] calldata priceIds, | ||
uint256 callbackGasLimit | ||
) external payable returns (uint64 sequenceNumber); | ||
|
||
/** | ||
* @notice Executes the callback for a price update request | ||
* @dev Requires 1.5x the callback gas limit to account for cross-contract call overhead | ||
* For example, if callbackGasLimit is 1M, the transaction needs at least 1.5M gas + some gas for some other operations in the function before the callback | ||
* @param sequenceNumber The sequence number of the request | ||
* @param updateData The raw price update data from Pyth | ||
* @param priceIds The price feed IDs to update, must match the request | ||
*/ | ||
function executeCallback( | ||
uint64 sequenceNumber, | ||
bytes[] calldata updateData, | ||
bytes32[] calldata priceIds | ||
) external payable; | ||
|
||
// Getters | ||
/** | ||
* @notice Gets the base fee charged by Pyth protocol | ||
* @dev This is a fixed fee per request that goes to the Pyth protocol, separate from gas costs | ||
* @return pythFeeInWei The base fee in wei that every request must pay | ||
*/ | ||
function getPythFeeInWei() external view returns (uint128 pythFeeInWei); | ||
|
||
/** | ||
* @notice Calculates the total fee required for a price update request | ||
* @dev Total fee = base Pyth protocol fee + gas costs for callback | ||
* @param callbackGasLimit The amount of gas allocated for callback execution | ||
* @return feeAmount The total fee in wei that must be provided as msg.value | ||
*/ | ||
function getFee( | ||
uint256 callbackGasLimit | ||
) external view returns (uint128 feeAmount); | ||
|
||
function getAccruedFees() external view returns (uint128 accruedFeesInWei); | ||
|
||
function getRequest( | ||
uint64 sequenceNumber | ||
) external view returns (PulseState.Request memory req); | ||
|
||
// Add these functions to the IPulse interface | ||
function setFeeManager(address manager) external; | ||
|
||
function withdrawAsFeeManager(uint128 amount) external; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,291 @@ | ||
// SPDX-License-Identifier: Apache 2 | ||
|
||
pragma solidity ^0.8.0; | ||
|
||
import "@openzeppelin/contracts/utils/math/SafeCast.sol"; | ||
import "@pythnetwork/pyth-sdk-solidity/IPyth.sol"; | ||
import "./IPulse.sol"; | ||
import "./PulseState.sol"; | ||
import "./PulseErrors.sol"; | ||
|
||
abstract contract Pulse is IPulse, PulseState { | ||
function _initialize( | ||
address admin, | ||
uint128 pythFeeInWei, | ||
address pythAddress, | ||
bool prefillRequestStorage | ||
) internal { | ||
require(admin != address(0), "admin is zero address"); | ||
require(pythAddress != address(0), "pyth is zero address"); | ||
|
||
_state.admin = admin; | ||
_state.accruedFeesInWei = 0; | ||
_state.pythFeeInWei = pythFeeInWei; | ||
_state.pyth = pythAddress; | ||
_state.currentSequenceNumber = 1; | ||
|
||
if (prefillRequestStorage) { | ||
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. what is this for :? 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. this is also referenced from the entropy contract -- which is to optimize for gas costs, by pre-filling the storage slots with non-zero values during initialization we pay the high gas cost once during deployment and hence subsequent writes to these slots will cost less gas |
||
for (uint8 i = 0; i < NUM_REQUESTS; i++) { | ||
Request storage req = _state.requests[i]; | ||
req.sequenceNumber = 0; | ||
req.publishTime = 1; | ||
req.callbackGasLimit = 1; | ||
req.requester = address(1); | ||
req.numPriceIds = 0; | ||
// Pre-warm the priceIds array storage | ||
for (uint8 j = 0; j < MAX_PRICE_IDS; j++) { | ||
req.priceIds[j] = bytes32(0); | ||
} | ||
} | ||
} | ||
} | ||
|
||
function requestPriceUpdatesWithCallback( | ||
uint256 publishTime, | ||
bytes32[] calldata priceIds, | ||
uint256 callbackGasLimit | ||
) external payable override returns (uint64 requestSequenceNumber) { | ||
// NOTE: The 60-second future limit on publishTime prevents a DoS vector where | ||
// attackers could submit many low-fee requests for far-future updates when gas prices | ||
// are low, forcing executors to fulfill them later when gas prices might be much higher. | ||
// Since tx.gasprice is used to calculate fees, allowing far-future requests would make | ||
// the fee estimation unreliable. | ||
require(publishTime <= block.timestamp + 60, "Too far in future"); | ||
cctdaniel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (priceIds.length > MAX_PRICE_IDS) { | ||
revert TooManyPriceIds(priceIds.length, MAX_PRICE_IDS); | ||
} | ||
requestSequenceNumber = _state.currentSequenceNumber++; | ||
|
||
uint128 requiredFee = getFee(callbackGasLimit); | ||
if (msg.value < requiredFee) revert InsufficientFee(); | ||
|
||
Request storage req = allocRequest(requestSequenceNumber); | ||
req.sequenceNumber = requestSequenceNumber; | ||
req.publishTime = publishTime; | ||
req.callbackGasLimit = callbackGasLimit; | ||
req.requester = msg.sender; | ||
req.numPriceIds = uint8(priceIds.length); | ||
|
||
// Copy price IDs to storage | ||
for (uint8 i = 0; i < priceIds.length; i++) { | ||
req.priceIds[i] = priceIds[i]; | ||
} | ||
|
||
_state.accruedFeesInWei += SafeCast.toUint128(msg.value); | ||
|
||
emit PriceUpdateRequested(req, priceIds); | ||
} | ||
|
||
function executeCallback( | ||
uint64 sequenceNumber, | ||
bytes[] calldata updateData, | ||
bytes32[] calldata priceIds | ||
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. Hmm thinking about it, now that we don't emit the ids and don't store them (we store the hash of it), how argus is supposed to understand it :? parse the transactions? 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. hmm i think we should still emit the event with the priceId and the trade-off of higher gas costs for event emission is necessary because the system can't function without Argus having access to the price IDs, we can keep the storage the hash still so that it remains optimized, what do you think? |
||
) external payable override { | ||
Request storage req = findActiveRequest(sequenceNumber); | ||
|
||
// Verify priceIds match | ||
require( | ||
priceIds.length == req.numPriceIds, | ||
"Price IDs length mismatch" | ||
); | ||
for (uint8 i = 0; i < req.numPriceIds; i++) { | ||
if (priceIds[i] != req.priceIds[i]) { | ||
revert InvalidPriceIds(priceIds[i], req.priceIds[i]); | ||
} | ||
} | ||
|
||
// Parse price feeds first to measure gas usage | ||
PythStructs.PriceFeed[] memory priceFeeds = IPyth(_state.pyth) | ||
.parsePriceFeedUpdates( | ||
cctdaniel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
updateData, | ||
priceIds, | ||
cctdaniel marked this conversation as resolved.
Show resolved
Hide resolved
|
||
SafeCast.toUint64(req.publishTime), | ||
SafeCast.toUint64(req.publishTime) | ||
); | ||
|
||
clearRequest(sequenceNumber); | ||
|
||
// Check if enough gas remains for callback + events/cleanup | ||
// We need extra gas beyond callbackGasLimit for: | ||
// 1. Emitting success/failure events | ||
// 2. Error handling in catch blocks | ||
// 3. State cleanup operations | ||
if (gasleft() < (req.callbackGasLimit * 3) / 2) { | ||
revert InsufficientGas(); | ||
} | ||
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. why do you need this check? Why not just run the rest of the code, and then either you run out of gas or you don't? 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 it just provides better UX by failing early with a clear error message, also actual gas paid is gas used there might be a situation where:
|
||
|
||
try | ||
IPulseConsumer(req.requester).pulseCallback{ | ||
gas: req.callbackGasLimit | ||
}(sequenceNumber, msg.sender, priceFeeds) | ||
{ | ||
// Callback succeeded | ||
emitPriceUpdate(sequenceNumber, priceIds, priceFeeds); | ||
} catch Error(string memory reason) { | ||
// Explicit revert/require | ||
emit PriceUpdateCallbackFailed( | ||
sequenceNumber, | ||
msg.sender, | ||
priceIds, | ||
req.requester, | ||
reason | ||
); | ||
} catch { | ||
// Out of gas or other low-level errors | ||
emit PriceUpdateCallbackFailed( | ||
sequenceNumber, | ||
msg.sender, | ||
priceIds, | ||
req.requester, | ||
"low-level error (possibly 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 don't think you want these catches either. If either of these errors happens for a transient reason, it means the callback can't be called in the future. In entropy if a callback fails, we leave the request open. It could get fulfilled later, or it could just sit there forever. 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. (this is actually important. In practice, we have callbacks that error on the first try and then succeed on a later retry) 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. Oh you also need to use the Check-Effects-Interactions pattern here, meaning the 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. the reason why we have this try-catch is because @m30m said that in entropy it's difficult to know if its the user error or our error so logging this will provide better UX, we can document that the callback contract should not fail, and if it fails we wont retry |
||
} | ||
|
||
function emitPriceUpdate( | ||
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. It's good to emit something (for debugging purposes), just be aware that it might be expensive to do this in ethereum mainnet. 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. agreed but apart from debugging this is also useful for off-chain indexing/integration so i think the benefits outweigh the costs |
||
uint64 sequenceNumber, | ||
bytes32[] memory priceIds, | ||
PythStructs.PriceFeed[] memory priceFeeds | ||
) internal { | ||
int64[] memory prices = new int64[](priceFeeds.length); | ||
uint64[] memory conf = new uint64[](priceFeeds.length); | ||
int32[] memory expos = new int32[](priceFeeds.length); | ||
uint256[] memory publishTimes = new uint256[](priceFeeds.length); | ||
|
||
for (uint i = 0; i < priceFeeds.length; i++) { | ||
prices[i] = priceFeeds[i].price.price; | ||
conf[i] = priceFeeds[i].price.conf; | ||
expos[i] = priceFeeds[i].price.expo; | ||
publishTimes[i] = priceFeeds[i].price.publishTime; | ||
} | ||
|
||
emit PriceUpdateExecuted( | ||
sequenceNumber, | ||
msg.sender, | ||
priceIds, | ||
prices, | ||
conf, | ||
expos, | ||
publishTimes | ||
); | ||
} | ||
|
||
function getFee( | ||
uint256 callbackGasLimit | ||
) public view override returns (uint128 feeAmount) { | ||
uint128 baseFee = _state.pythFeeInWei; | ||
uint256 gasFee = callbackGasLimit * tx.gasprice; | ||
feeAmount = baseFee + SafeCast.toUint128(gasFee); | ||
} | ||
Comment on lines
+173
to
+179
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'm wondering how this works in Entropy. Do we update pythFee regularly based on gasPrice? @m30m might know better but similar products charge fee as % of the tx fee. 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. There is also one DoS attack vector in our price feeds use case regarding gas prices. People can ask for price updates anytime in the future and it won't be fullfilled immediately (opposed to Entropy) and this might make My recommendation is to not allow a publish time that is more than 1min in the future. Or charge higher fees as it goes more into 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. yeah that's a good point, i think not allowing a publish time more than 1 min in the future is a reasonable solution 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. In Entropy, the keeper updates its fee over time. I think the thing to do here is to give the keeper a configurable |
||
|
||
function getPythFeeInWei() | ||
public | ||
view | ||
override | ||
returns (uint128 pythFeeInWei) | ||
{ | ||
pythFeeInWei = _state.pythFeeInWei; | ||
} | ||
|
||
function getAccruedFees() | ||
public | ||
view | ||
override | ||
returns (uint128 accruedFeesInWei) | ||
{ | ||
accruedFeesInWei = _state.accruedFeesInWei; | ||
} | ||
|
||
function getRequest( | ||
uint64 sequenceNumber | ||
) public view override returns (Request memory req) { | ||
req = findRequest(sequenceNumber); | ||
} | ||
|
||
function requestKey( | ||
uint64 sequenceNumber | ||
) internal pure returns (bytes32 hash, uint8 shortHash) { | ||
hash = keccak256(abi.encodePacked(sequenceNumber)); | ||
shortHash = uint8(hash[0] & NUM_REQUESTS_MASK); | ||
} | ||
|
||
function withdrawFees(uint128 amount) external { | ||
require(msg.sender == _state.admin, "Only admin can withdraw fees"); | ||
require(_state.accruedFeesInWei >= amount, "Insufficient balance"); | ||
|
||
_state.accruedFeesInWei -= amount; | ||
|
||
(bool sent, ) = msg.sender.call{value: amount}(""); | ||
require(sent, "Failed to send fees"); | ||
|
||
emit FeesWithdrawn(msg.sender, amount); | ||
} | ||
|
||
function findActiveRequest( | ||
uint64 sequenceNumber | ||
) internal view returns (Request storage req) { | ||
req = findRequest(sequenceNumber); | ||
|
||
if (!isActive(req) || req.sequenceNumber != sequenceNumber) | ||
revert NoSuchRequest(); | ||
} | ||
|
||
function findRequest( | ||
uint64 sequenceNumber | ||
) internal view returns (Request storage req) { | ||
(bytes32 key, uint8 shortKey) = requestKey(sequenceNumber); | ||
|
||
req = _state.requests[shortKey]; | ||
if (req.sequenceNumber == sequenceNumber) { | ||
return req; | ||
} else { | ||
req = _state.requestsOverflow[key]; | ||
} | ||
} | ||
|
||
function clearRequest(uint64 sequenceNumber) internal { | ||
(bytes32 key, uint8 shortKey) = requestKey(sequenceNumber); | ||
|
||
Request storage req = _state.requests[shortKey]; | ||
if (req.sequenceNumber == sequenceNumber) { | ||
req.sequenceNumber = 0; | ||
} else { | ||
delete _state.requestsOverflow[key]; | ||
} | ||
} | ||
|
||
function allocRequest( | ||
uint64 sequenceNumber | ||
) internal returns (Request storage req) { | ||
(, uint8 shortKey) = requestKey(sequenceNumber); | ||
|
||
req = _state.requests[shortKey]; | ||
if (isActive(req)) { | ||
(bytes32 reqKey, ) = requestKey(req.sequenceNumber); | ||
_state.requestsOverflow[reqKey] = req; | ||
} | ||
} | ||
|
||
function isActive(Request storage req) internal view returns (bool) { | ||
return req.sequenceNumber != 0; | ||
} | ||
|
||
function setFeeManager(address manager) external override { | ||
require(msg.sender == _state.admin, "Only admin can set fee manager"); | ||
address oldFeeManager = _state.feeManager; | ||
_state.feeManager = manager; | ||
emit FeeManagerUpdated(_state.admin, oldFeeManager, manager); | ||
} | ||
|
||
function withdrawAsFeeManager(uint128 amount) external override { | ||
require(msg.sender == _state.feeManager, "Only fee manager"); | ||
require(_state.accruedFeesInWei >= amount, "Insufficient balance"); | ||
|
||
_state.accruedFeesInWei -= amount; | ||
|
||
(bool sent, ) = msg.sender.call{value: amount}(""); | ||
require(sent, "Failed to send fees"); | ||
|
||
emit FeesWithdrawn(msg.sender, amount); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
// SPDX-License-Identifier: Apache 2 | ||
|
||
pragma solidity ^0.8.0; | ||
|
||
error NoSuchProvider(); | ||
error NoSuchRequest(); | ||
error InsufficientFee(); | ||
error Unauthorized(); | ||
error InvalidCallbackGas(); | ||
error CallbackFailed(); | ||
error InvalidPriceIds(bytes32 providedPriceIdsHash, bytes32 storedPriceIdsHash); | ||
error InvalidCallbackGasLimit(uint256 requested, uint256 stored); | ||
error ExceedsMaxPrices(uint32 requested, uint32 maxAllowed); | ||
error InsufficientGas(); | ||
error TooManyPriceIds(uint256 provided, uint256 maximum); |
Uh oh!
There was an error while loading. Please reload this page.