-
Notifications
You must be signed in to change notification settings - Fork 209
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
Implemented a Math Library to combine Pyth prices in Solidity - tested and working on Remix IDE #1732
Implemented a Math Library to combine Pyth prices in Solidity - tested and working on Remix IDE #1732
Changes from all commits
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 |
---|---|---|
@@ -0,0 +1,167 @@ | ||
// PriceCombine.sol | ||
// SPDX-License-Identifier: Apache 2 | ||
pragma solidity ^0.8.0; | ||
|
||
import "./PythStructs.sol"; | ||
|
||
// Constants for working with Pyth's number representation | ||
int32 constant PD_EXPO = -9; | ||
uint64 constant PD_SCALE = 1_000_000_000; | ||
uint64 constant MAX_PD_V_U64 = (1 << 28) - 1; | ||
|
||
// Solidity Library to easily combine 2 Pyth Prices into 1 ( for example SOL:USD/ETH:USD=SOL/ETH). For Rust implementation, refer Pyth Rust SDK https://github.com/pyth-network/pyth-sdk-rs/blob/main/pyth-sdk/src/price.rs | ||
// Based on the Rust implementation, the functions are defined within the `impl Price` block, and they are public methods of the `Price` struct and are as follows `get_price_in_quote`, `div`, `normalize`, `scale_to_exponent` | ||
// Similarly the functions implemented in solidity are `public` and `pure` types. | ||
|
||
library PriceCombine { | ||
/** | ||
* @notice Computes the price in quote currency. | ||
* @param self The price of the base currency. | ||
* @param quote The price of the quote currency. | ||
* @param resultExpo The exponent for the result. | ||
* @return The combined price in the quote currency. | ||
*/ | ||
function getPriceInQuote(PythStructs.Price memory self, PythStructs.Price memory quote, int32 resultExpo) public pure returns (PythStructs.Price memory) { | ||
PythStructs.Price memory result = div(self, quote); | ||
// Return zero price so the error can be gracefully handled by the caller | ||
if (result.price == 0 && result.conf == 0) { | ||
return PythStructs.Price({ | ||
price: 0, | ||
conf: 0, | ||
expo: 0, | ||
publishTime: 0 | ||
}); | ||
} | ||
return scaleToExponent(result, resultExpo); | ||
} | ||
|
||
/** | ||
* @notice Divides the price of the base currency by the quote currency price. | ||
* @param self The price of the base currency. | ||
* @param other The price of the quote currency. | ||
* @return The combined price after division. | ||
*/ | ||
function div(PythStructs.Price memory self, PythStructs.Price memory other) public pure returns (PythStructs.Price memory) { | ||
PythStructs.Price memory base = normalize(self); | ||
other = normalize(other); | ||
|
||
// If the price of the quote currency is zero, return zero | ||
if (other.price == 0) { | ||
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 would suggest you throw an error here or return |
||
return PythStructs.Price({ | ||
price: 0, | ||
conf: 0, | ||
expo: 0, | ||
publishTime: 0 | ||
}); | ||
} | ||
|
||
// Convert prices to unsigned integers and get their signs | ||
(uint64 basePrice, int64 baseSign) = toUnsigned(base.price); | ||
(uint64 otherPrice, int64 otherSign) = toUnsigned(other.price); | ||
|
||
// Compute the midprice | ||
uint64 midprice = basePrice * PD_SCALE / otherPrice; | ||
int32 midpriceExpo = base.expo - other.expo + PD_EXPO; | ||
|
||
// Compute the confidence interval | ||
uint64 otherConfidencePct = other.conf * PD_SCALE / otherPrice; | ||
uint128 conf = (base.conf * PD_SCALE / otherPrice) + (otherConfidencePct * midprice) / PD_SCALE; | ||
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 should be dividing the sum by 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. also i presume you want to cast each of the summands to 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. However, if you want to ensure that conf is less than the max 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 will make this change |
||
|
||
// Check for overflow and return the result | ||
if (conf < type(uint64).max) { | ||
return PythStructs.Price( | ||
int64(int64(midprice) * baseSign * otherSign), | ||
uint64(conf), | ||
midpriceExpo, | ||
self.publishTime < other.publishTime ? self.publishTime : other.publishTime | ||
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. you can use OpenZeppelin's Math library for better readability: https://docs.openzeppelin.com/contracts/3.x/api/math#Math-min-uint256-uint256- 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. Alright, I will look into this |
||
); | ||
} else { | ||
// Return zero price if there's an overflow | ||
return PythStructs.Price({ | ||
price: 0, | ||
conf: 0, | ||
expo: 0, | ||
publishTime: 0 | ||
}); | ||
} | ||
} | ||
|
||
/** | ||
* @notice Normalizes the price and confidence to be within acceptable range. | ||
* @param self The price structure to normalize. | ||
* @return The normalized price structure. | ||
*/ | ||
function normalize(PythStructs.Price memory self) public pure returns (PythStructs.Price memory) { | ||
(uint64 price, int64 sign) = toUnsigned(self.price); | ||
uint64 conf = self.conf; | ||
int32 expo = self.expo; | ||
|
||
// Adjust the price and confidence if they are too large | ||
while (price > MAX_PD_V_U64 || conf > MAX_PD_V_U64) { | ||
price = price / 10; | ||
conf = conf / 10; | ||
expo = expo + 1; | ||
} | ||
|
||
return PythStructs.Price({ | ||
price: int64(price) * sign, | ||
conf: conf, | ||
expo: expo, | ||
publishTime: self.publishTime | ||
}); | ||
} | ||
|
||
/** | ||
* @notice Scales the price to the target exponent. | ||
* @param self The price structure to scale. | ||
* @param targetExpo The target exponent. | ||
* @return The scaled price structure. | ||
*/ | ||
function scaleToExponent(PythStructs.Price memory self, int32 targetExpo) public pure returns (PythStructs.Price memory) { | ||
int32 delta = targetExpo - self.expo; | ||
int64 price = self.price; | ||
uint64 conf = self.conf; | ||
|
||
// Adjust the price and confidence based on the exponent difference | ||
if (delta >= 0) { | ||
while (delta > 0 && (price != 0 || conf != 0)) { | ||
price = price / 10; | ||
conf = conf / 10; | ||
delta = delta - 1; | ||
} | ||
return PythStructs.Price({ | ||
price: price, | ||
conf: conf, | ||
expo: targetExpo, | ||
publishTime: self.publishTime | ||
}); | ||
} else { | ||
while (delta < 0) { | ||
price = price * 10; | ||
conf = conf * 10; | ||
delta = delta + 1; | ||
} | ||
return PythStructs.Price({ | ||
price: price, | ||
conf: conf, | ||
expo: targetExpo, | ||
publishTime: self.publishTime | ||
}); | ||
} | ||
} | ||
|
||
/** | ||
* @notice Converts a signed integer to an unsigned integer and a sign bit. | ||
* @param x here is the signed integer. | ||
* @return The unsigned integer and sign bit. | ||
*/ | ||
function toUnsigned(int64 x) public pure returns (uint64, int64) { | ||
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 function can be internal. is it intended to be used for anything other than a helper method within this library? 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 agree, we can keep it |
||
if (x == type(int64).min) { | ||
return (uint64(type(int64).max) + 1, -1); | ||
} else if (x < 0) { | ||
return (uint64(-x), -1); | ||
} else { | ||
return (uint64(x), 1); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,34 @@ | ||||||||
// SPDX-License-Identifier: Apache 2 | ||||||||
pragma solidity ^0.8.0; | ||||||||
|
||||||||
import "./PythStructs.sol"; | ||||||||
import "./PriceCombine.sol"; | ||||||||
|
||||||||
// This contract is used to test the library functions and tested in Remix IDE. All ABIs are present in the /abis folder | ||||||||
contract PriceCombineTest { | ||||||||
using PriceCombine for PythStructs.Price; | ||||||||
|
||||||||
/** | ||||||||
* @notice Test the getPriceInQuote function | ||||||||
* @param basePrice The price of the base currency (example: SOL/USD) 13913000000 (represents the current SOL/USD $139.13 with 8 decimal places) | ||||||||
* @param baseConf The confidence interval of the base currency | ||||||||
* @param baseExpo The exponent of the base currency (here: -8 -> 8 decimal units) indicates price is scaled by 10^-8 | ||||||||
* @param basePublishTime The publish time of the base currency (UNIX timestamp) | ||||||||
* @param quotePrice The price of the quote currency (example: ETH/USD) 341853000000 (represents the current ETH/USD $3418.53 with 8 decimal places) | ||||||||
* @param quoteConf The confidence interval of the quote currency | ||||||||
* @param quoteExpo The exponent of the quote currency (here: -8 -> 8 decimal units) indicates price is scaled by 10^-8 | ||||||||
* @param quotePublishTime The publish time of the quote currency (UNIX timestamp) | ||||||||
* @param resultExpo The desired exponent for the result (here: -8) | ||||||||
* @return The price, confidence interval, exponent, and publish time of the resulting price | ||||||||
*/ | ||||||||
function testGetPriceInQuote(int64 basePrice, uint64 baseConf, int32 baseExpo, uint basePublishTime, | ||||||||
int64 quotePrice, uint64 quoteConf, int32 quoteExpo, uint quotePublishTime, | ||||||||
int32 resultExpo) public pure returns (int64, uint64, int32, uint) { | ||||||||
PythStructs.Price memory base = PythStructs.Price(basePrice, baseConf, baseExpo, basePublishTime); | ||||||||
PythStructs.Price memory quote = PythStructs.Price(quotePrice, quoteConf, quoteExpo, quotePublishTime); | ||||||||
PythStructs.Price memory result = base.getPriceInQuote(quote, resultExpo); | ||||||||
return (result.price, result.conf, result.expo, result.publishTime); | ||||||||
} | ||||||||
|
||||||||
// Add more test functions as needed | ||||||||
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. maybe make a forge project and add test cases there? There should be a lot more tests before this can be merged 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. forge allows for fuzz testing as well which can be useful so that you don't have to manually define all the cases 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 see. Can you provide me more details on forge? Do the forge tests also need to be part of this codebase? 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. https://book.getfoundry.sh/reference/forge/forge you should be able to include this library in a forge environment and add tests there. there should be tests somewhere before this is merged. you can add tests to the pyth-crosschain/target_chains/ethereum/contracts/foundry.toml Lines 6 to 8 in 3cb45ec
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. Got it, thanks for the info |
||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,4 +30,4 @@ contract PythStructs { | |
// Latest available exponentially-weighted moving average price | ||
Price emaPrice; | ||
} | ||
} | ||
} | ||
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. can you remove this change? 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. sure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not put this check in
scaleToExponent
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will try to update the check in
scaleToExponent