Skip to content
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.

Sign up for GitHub

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

Review Comments #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Review Comments #1

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion contracts/PoolStateHelper.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ interface ISMAOracle is IOracleWrapper {
interface IPoolStateHelper {
error INVALID_PERIOD();

// todo should this include pending mints into this side?
struct SideInfo {
uint256 supply; // poolToken.totalSupply()
uint256 settlementBalance; // balance of settlementTokens associated with supply
Expand Down Expand Up @@ -120,10 +121,11 @@ contract PoolStateHelper is IPoolStateHelper {
uint128 public constant SHORT_INDEX = 1;

/**
* @notice Get an array of TotalCommitment in ascending order for a given period.
* @notice Get an array of TotalCommitment in ascending order for a given number of periods.
* @return commitQueue
* @param committer The PoolCommitter contract.
* @param periods The number of commits to get.
* @dev a committment period is defined by the update interval. That is one period is one update interval.
*/
function getCommitQueue(IPoolCommitter2 committer, uint256 periods)
public
Expand All @@ -136,6 +138,7 @@ contract PoolStateHelper is IPoolStateHelper {

unchecked {
for (uint256 i; i < periods; i++) {
// fetch the aggregate commitments for this period
commitQueue[i] = committer.totalPoolCommitments(
currentUpdateIntervalId + i
);
Expand Down Expand Up @@ -166,6 +169,9 @@ contract PoolStateHelper is IPoolStateHelper {
}

for (uint256 i = _i; i < _periodCount; i++) {
// index in either from 0 or from an offset of numPeriods - periodCount - 1
// Note: Should the index of smaInfo.prices not just start from 0? does this not just want to return an array
// of size _numPeriods - 1 OR _periodCount?
smaInfo.prices[
_periodCount < _numPeriods
? i
Expand Down Expand Up @@ -264,6 +270,7 @@ contract PoolStateHelper is IPoolStateHelper {
poolStateSnapshot.spotPrice
);

// Execute given update interval commitments
PoolInfo memory newPoolInfo = executeGivenCommit(
poolStateSnapshot.commitQueue[poolStateSnapshot.pointer],
calculateValueTransfer(
Expand Down Expand Up @@ -292,6 +299,7 @@ contract PoolStateHelper is IPoolStateHelper {
// Base case
finalExpectedPoolState = ExpectedPoolState({
cumulativePendingMintSettlement: newPendingSettlement,
// note: why do we multiply out by decimals here? Is that not already factored into the settlement balance?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to integer division - there can be a scenario where long.settlementBalance < short.settlementBalance. Scaling by settlementTokenDecimals seemed a logical choice to me because shortPrice and longPrice are scaled the same way.

skew: (newPoolInfo.long.settlementBalance *
10**poolStateSnapshot.settlementTokenDecimals) /
newPoolInfo.short.settlementBalance,
Expand Down