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.

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

Add draft StickyOracle #2

Draft
wants to merge 19 commits into
base: dev
Choose a base branch
from
Draft

Add draft StickyOracle #2

wants to merge 19 commits into from

Conversation

telome
Copy link
Contributor

@telome telome commented Sep 25, 2023

No description provided.

@telome telome marked this pull request as draft September 25, 2023 14:04
src/StickyOracle.sol Outdated Show resolved Hide resolved
src/StickyOracle.sol Outdated Show resolved Hide resolved
src/StickyOracle.sol Outdated Show resolved Hide resolved
return type(uint128).max; // TODO: consider better fallback for missing accums
}

function poke() public {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
function poke() public {
function poke() external {

src/StickyOracle.sol Outdated Show resolved Hide resolved
age = uint32(block.timestamp);
}

function read() external view toll returns (uint128) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

To keep consistency with the peek function we should add a require that cur > 0 in this one, or just return true in peek always.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I also think that peek should probably use pip.peek(), so we make sure it won't revert.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes peek() should be calling pip.peek(), but since pip.read() already reverts when cur is 0 I guess we don't need a require in read().

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe that is assuming too much from the called oracle but yeah not sure, will think about this a bit more.


uint256 val_ = val;
require(val_ > 0, "StickyOracle/not-init");
return uint128(val_ * slope_ / RAY); // fallback for missing accumulators
Copy link
Contributor

Choose a reason for hiding this comment

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

Also - maybe it' simpler just to use the last val in this case and not multiply by slope?

accumulators[day] = acc1 + (acc1 - acc2) * i / (j - i);
}

function poke() external {
Copy link
Contributor

@oldchili oldchili Dec 4, 2023

Choose a reason for hiding this comment

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

Wouldn't it be much simpler just to store the timestamp and value of the last first-of-the-day poke? I feel like we are trying to be too smart here with the bef and aft calculations.

I tried to sketch how it would look like (also with saving cap in storage to avoid calculating it on each read/pip):

    function poke()  {
        uint256 today = block.timestamp / 1 days;
        AccData memory accToday = accumulators[today];
        bool firstPokeToday = accToday.ts == 0;

        // get or recompute cap
        uint128 cap_;
        if (firstPokeToday) {
            cap_ = _getCap();
            cap = cap_;
        } else cap_ = cap;

        // update oracle value
        uint128 cur = _min(pip.read(), cap_);
        val = cur;
        age = uint32(block.timestamp);

        // update accumulator if needed
        if (firstPokeToday) {
            AccData memory accLast_ = accLast; // this is from storage

            accToday.val = _accLast.val + cur * (block.timestamp - _accLast.ts);
            accToday.ts = uint32(block.timestamp);

            accumulators[today] = accToday;
            accLast = accToday;
        }
    }

The above assumes we have in storage accLast and cap, where each accumulator entry is a struct with a value and a timestamp.

uint256 tmrTs = (today + 1) * 1 days; // timestamp on the first second of tomorrow
if (acc == 0) { // first poke of the day
uint256 prevDay = age_ / 1 days;
uint256 bef = val_ * (block.timestamp - (prevDay + 1) * 1 days); // contribution to the accumulator from the previous value
Copy link
Contributor

Choose a reason for hiding this comment

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

is val_ necessarily the previous day's accumulator value? not sure I get this.

uint256 aft = cur * (tmrTs - block.timestamp); // contribution to the accumulator from the current value, optimistically assuming this will be the last poke of the day
newAcc = accumulators[prevDay] + bef + aft;
} else { // not the first poke of the day
uint256 off = tmrTs - block.timestamp; // period during which the accumulator value needs to be adjusted
Copy link
Contributor

Choose a reason for hiding this comment

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

mmm can we do without the else case? we probably don't want twap related logic on each regular poke.

}

function read() external view toll returns (uint128) {
return _min(pip.read(), _getCap());
Copy link
Contributor

Choose a reason for hiding this comment

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

_getCap is pretty heavy to calculate on each oracle read, maybe we can store the result in storage once a day (I tried to to that with my sketching of poke() above).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants