fix(funding-rate-proposer): Use historical-price to propose funding rates #2881
Conversation
…ates - Proposal uses the latest block timestamp as the request timestamp. Any disputes will reference this same timestamp, therefore the proposed price should be the historical price for this timestamp. Currently, the current price is used, but the current price can be off from the historical price at the request timestamp because of the lag between the last block time and the current (off-chain) system time.
| // Get hypothetical timestamp to use for a new price proposal: | ||
| // Unless `usePriceFeedTime=true`, use the latest block's timestamp as the request | ||
| // timestamp so that the contract does not interpret `requestTimestamp` as being in the future. | ||
| const requestTimestamp = usePriceFeedTime |
There was a problem hiding this comment.
This requestTimestamp logic is copied from below, line 198ish
| @@ -163,8 +163,33 @@ class FundingRateProposer { | |||
|
|
|||
| // Assume pricefeed has been cached and updated prior to this function via the `update()` call. | |||
| const priceFeed = this.priceFeedCache[fundingRateIdentifier]; | |||
| const pricefeedPrice = await this._getPriceFeedAndPrice(fundingRateIdentifier); | |||
| if (!pricefeedPrice) return; | |||
| if (!priceFeed) { | |||
There was a problem hiding this comment.
This checking for !priceFeed is copied from below, line 397ish
| ? priceFeed.getLastUpdateTime() | ||
| : (await this.web3.eth.getBlock("latest")).timestamp; | ||
| let _pricefeedPrice; | ||
| try { |
There was a problem hiding this comment.
This try-catch is the new logic in that it computes a historical (funding rate) price instead of the current price.
| // timestamp so that the contract does not interpret `requestTimestamp` as being in the future. | ||
| const requestTimestamp = usePriceFeedTime | ||
| ? priceFeed.getLastUpdateTime() | ||
| : (await this.web3.eth.getBlock("latest")).timestamp; |
There was a problem hiding this comment.
Is it a problem if the latest block time is > the price feed's latest update time?
There was a problem hiding this comment.
Yes theoretically this could cause divergence in historical price fetched for this request time by a disputer. We could conservatively add an assert(await this.web3.eth.getBlock("latest")).timestamp <= priceFeed.getLastUpdateTime()) here and just update the pricefeed again and check the new time. WDYT?
There was a problem hiding this comment.
Or we can do like the optimistic-oracle-proposer and just conservatively update() the priceFeed before fetching historical price. reference
There was a problem hiding this comment.
I implemented the above ^
There was a problem hiding this comment.
Sounds good! This isn't guaranteed to have an update time > block time, but I think this is probably as close as we can reasonably get:
- Price feeds usually throttle to ensure they don't update more than once per minute by default.
- Block times can (but usually don't) be ahead of wall-clock time (they have a 90 second or so tolerance if I remember correctly).
There was a problem hiding this comment.
yeah the other solution which I think is a bit over engineered for now is something like
let requestTimestamp;
// Update pricefeed until its last update time is caught up with the latest network block, or
// break early after 1 minute and assume that's good enough (or throw).
setTimeout(function() {
while(true) {
await priceFeed.update();
requestTimestamp = (await web3.eth.getBlock()).timestamp
if (priceFeed.lastUpdateTime() >= requestTimestamp) break;
}
},
60000There was a problem hiding this comment.
Yeah, agree with your decision to do it the way you have it in the PR. LGTM!
There was a problem hiding this comment.
But this wouldn't be useful unless the minTimeBetweenUpdates was respected, and given the expected iteration cadence of 5 minutes we don't have much timeout period to play with. This sort of method would make sense in a looping infrastructure
Motivation
Funding Rate Proposer bot uses the latest block timestamp as the request timestamp. Any disputes will reference this same timestamp, therefore the proposed price should be the historical price for this timestamp. Currently, the current price is used, but the current price can be off from the historical price at the request timestamp because of the lag between the last block time and the current (off-chain) system time.
The latest block timestamp is used as the request time instead of the current system time because the Optimistic Oracle prevents request timestamps from being >= the latest block time. See this PR for more details.
Testing
Check a box to describe how you tested these changes and list the steps for reviewers to test.