more stuff

Author: jayantk

target_chains/ethereum/contracts/contracts/pulse/IPulse.sol

@@ -6,11 +6,32 @@ import "@pythnetwork/pyth-sdk-solidity/IPyth.sol";
 import "./PulseEvents.sol";
 import "./PulseState.sol";
 
-interface IPulseConsumer {
+abstract contract IPulseConsumer {
+    // This method is called by Pulse to provide the price updates to the consumer.
+    // It asserts that the msg.sender is the Pulse contract. It is not meant to be
+    // overridden by the consumer.
+    function _pulseCallback(
+        uint64 sequenceNumber,
+        PythStructs.PriceFeed[] memory priceFeeds
+    ) external {
+        address pulse = getPulse();
+        require(pulse != address(0), "Pulse address not set");
+        require(msg.sender == pulse, "Only Pulse can call this function");
+
+        pulseCallback(sequenceNumber, priceFeeds);
+    }
+
+    // getPulse returns the Pulse contract address. The method is being used to check that the
+    // callback is indeed from the Pulse contract. The consumer is expected to implement this method.
+    function getPulse() internal view virtual returns (address);
+
+    // This method is expected to be implemented by the consumer to handle the price updates.
+    // It will be called by _pulseCallback after _pulseCallback ensures that the call is
+    // indeed from Pulse contract.
     function pulseCallback(
         uint64 sequenceNumber,
         PythStructs.PriceFeed[] memory priceFeeds
-    ) external;
+    ) internal virtual;
 }
 
 interface IPulse is PulseEvents {

target_chains/ethereum/contracts/contracts/pulse/Pulse.sol

@@ -99,7 +99,7 @@ abstract contract Pulse is IPulse, PulseState {
         emit PriceUpdateRequested(req, priceIds);
     }
 
-    // TODO: I don't think this should be payable. Any cost paid to Pyth should come from the contract and be taken out of the provider's accrued fees.
+    // TODO: does this need to be payable? Any cost paid to Pyth could be taken out of the provider's accrued fees.
     function executeCallback(
         address providerToCredit,
         uint64 sequenceNumber,
@@ -145,12 +145,16 @@ abstract contract Pulse is IPulse, PulseState {
         clearRequest(sequenceNumber);
         // TODO: if this effect occurs here, we need to guarantee that executeCallback can never revert.
         // If executeCallback can revert, then funds can be permanently locked in the contract.
+        // TODO: there also needs to be some penalty mechanism in case the expected provider doesn't execute the callback.
+        // This should take funds from the expected provider and give to providerToCredit. The penalty should probably scale
+        // with time in order to ensure that the callback eventually gets executed.
+        // (There may be exploits with ^ though if the consumer contract is malicious ?)
         _state.providers[providerToCredit].accruedFeesInWei += req.fee;
         _state.providers[providerToCredit].accruedFeesInWei += SafeCast
             .toUint128(msg.value - pythFee);
 
         try
-            IPulseConsumer(req.requester).pulseCallback{
+            IPulseConsumer(req.requester)._pulseCallback{
                 gas: req.callbackGasLimit
             }(sequenceNumber, priceFeeds)
         {

target_chains/ethereum/contracts/contracts/pulse/PulseState.sol

@@ -12,6 +12,7 @@ contract PulseState {
     struct Request {
         uint64 sequenceNumber;
         uint256 publishTime;
+        // TODO: this is going to absolutely explode gas costs. Need to do something smarter here.
         bytes32[MAX_PRICE_IDS] priceIds;
         uint8 numPriceIds; // Actual number of price IDs used
         uint256 callbackGasLimit;