7266 auction status #7339
7266 auction status #7339
Conversation
Chris-Hibbert
added
Inter-protocol
Chris-Hibbert
force-pushed
the
7266-auctionStatus
branch
from
2a696f4 to
c93516e
Compare
| const scheduler = await makeScheduler( | ||
| driver, | ||
| timer, | ||
| // @ts-expect-error types are correct. How to convince TS? |
There was a problem hiding this comment.
glad to see the suppression more targeted. I'll take a look at this types after pushing this review.
| /* | ||
| * If stateVariable is unset, set it to amount, otherwise add amount | ||
| */ |
There was a problem hiding this comment.
addAmountToState sounds like it's adding a property to the state. this is increasing an amount stored in state. The laziness of setting it if it isn't set seems unnecessarily dynamic. Why not set them to empty amounts during initState?
Then you don't even need this function. The two places it's called can be like,
state.remainingProceedsGoal = AmountMath.add(state.remainingProceedsGoal, incrementToGoal);which is statically analyzable. If you really want to avoid repeating the state property we could have a helper like,
increaseAmount(state, 'remainingProceedsGoal', incrementToGoal);… and have the type checker verify that the second arg is a property in the first arg, but I think that still breaks things like "find all references".
Kind of a general challenge with immutable data when you want to change it without repeating the name of the thing being changed.
There was a problem hiding this comment.
A couple of these values distinguish between empty and null.
There was a problem hiding this comment.
I removed the helper. I considered renaming to addToNullable(), since state was the first arg anyway, but your comments about type checking and static analysis convinced me the function wasn't adding enough value.
| state.startCollateral = null; | ||
| state.lockedPriceForRound = null; | ||
| state.curAuctionPrice = null; | ||
| state.remainingProceedsGoal = null; |
There was a problem hiding this comment.
that the nullness of these four values has to be synchronized worries me. if they're really non or not at the same time, wouldn't it be better to bundle them in an object? then "between auctions" that single object ref is null and otherwise it's populated in a verifiable "during auction" shape
There was a problem hiding this comment.
They're not as parallel as they looked. After looking more closely,
- startCollateral will never be null. it's reset to empty at the end of an auction, and increases when
addAssets()is called startProceedsGoalis reset at the end of an auction. it increases whenaddAssets()is called with a goal.lockedPriceForRoundis assigned a value to lock the price, and reset to null at the end of the auction.curAuctionPriceis assigned a value at the beginning of each descending step, and reset at the end of the auction.remainingProceedsGoalis assigned a value at the beginning of the auction (if there's a goal), reduced on every trade, and reset at the end of the auction.
| /** @returns {Promise<{now:Timestamp, nextSchedule: Schedule}>} */ | ||
| const initializeNextSchedule = async () => { | ||
| return E.when( | ||
| // XXX manualTimer returns a bigint, not a timeRecord. |
There was a problem hiding this comment.
exploration into burning this debt #7348
| }; | ||
| let { now, nextSchedule } = await initializeNextSchedule(); | ||
| const setTimeMonotonically = time => { | ||
| TimeMath.compareAbs(time, now) >= 0 || Fail`Time only moves forward`; |
|
@Chris-Hibbert - This PR appears to be stuck. It's had a merge label for > 24 hours |
Chris-Hibbert
force-pushed
the
7266-auctionStatus
branch
from
d531736 to
c634371
Compare
share 'now' across scheduler
Chris-Hibbert
force-pushed
the
7266-auctionStatus
branch
from
c634371 to
5315e6f
Compare
closes: #7266
Description
Write important data about the auction's status to chain storage.
Security Considerations
None
Scaling Considerations
Reduces load on the chain
Documentation Considerations
Added Auction data to the README. (replacing obsolete AMM info) I dunno where else it needs to be written up.
Testing Considerations
New Tests.