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

fix(funding-rate-proposer): Use historical-price to propose funding rates #2881

Merged
merged 4 commits into from
Apr 20, 2021
Merged

fix(funding-rate-proposer): Use historical-price to propose funding rates #2881

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
20a532a
fix(funding-rate-proposer): Use historical-price to propose funding r…
nicholaspai Apr 19, 2021
0854feb
Fix tests
nicholaspai Apr 19, 2021
f1d31e7
Update proposer.js
nicholaspai Apr 19, 2021
04a031c
Update pricefeed before fetching historical price
nicholaspai Apr 19, 2021
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
60 changes: 30 additions & 30 deletions packages/funding-rate-proposer/src/proposer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This checking for !priceFeed is copied from below, line 397ish

this.logger.error({
at: "PerpetualProposer",
message: "Failed to create pricefeed for funding rate identifier",
fundingRateIdentifier
});
return null;
}
// Get hypothetical timestamp to use for a new price proposal:
// Unless `usePriceFeedTime=true`, use the latest block's timestamp as the request
nicholaspai marked this conversation as resolved.
Show resolved Hide resolved
// timestamp so that the contract does not interpret `requestTimestamp` as being in the future.
const requestTimestamp = usePriceFeedTime
Copy link
Member Author

Choose a reason for hiding this comment

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

This requestTimestamp logic is copied from below, line 198ish

? priceFeed.getLastUpdateTime()
: (await this.web3.eth.getBlock("latest")).timestamp;
Copy link
Member

Choose a reason for hiding this comment

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

Is it a problem if the latest block time is > the price feed's latest update time?

Copy link
Member Author

Choose a reason for hiding this comment

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

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

Or we can do like the optimistic-oracle-proposer and just conservatively update() the priceFeed before fetching historical price. reference

Copy link
Member Author

Choose a reason for hiding this comment

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

I implemented the above ^

Copy link
Member

Choose a reason for hiding this comment

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

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:

  1. Price feeds usually throttle to ensure they don't update more than once per minute by default.
  2. 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).

Copy link
Member Author

Choose a reason for hiding this comment

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

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;
    }
},
60000

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, agree with your decision to do it the way you have it in the PR. LGTM!

Copy link
Member Author

Choose a reason for hiding this comment

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

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

let _pricefeedPrice;
try {
Copy link
Member Author

Choose a reason for hiding this comment

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

This try-catch is the new logic in that it computes a historical (funding rate) price instead of the current price.

_pricefeedPrice = (await priceFeed.getHistoricalPrice(Number(requestTimestamp))).toString();
} catch (error) {
this.logger.error({
at: "PerpetualProposer",
nicholaspai marked this conversation as resolved.
Show resolved Hide resolved
message: "Failed to query current price for funding rate identifier",
fundingRateIdentifier,
requestTimestamp
});
return;
}
const pricefeedPrice = this.formatPriceToPricefeedPrecision(_pricefeedPrice, priceFeed);
let onchainFundingRate = currentFundingRateData.rate.toString();

// Check that pricefeedPrice is within [configStore.minFundingRate, configStore.maxFundingRate]
Expand Down Expand Up @@ -195,11 +220,6 @@ class FundingRateProposer {
this.toBN(this.toWei(this.fundingRateErrorPercent.toString()))
);
if (shouldUpdateFundingRate) {
// 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
? priceFeed.getLastUpdateTime()
: (await this.web3.eth.getBlock("latest")).timestamp;
const proposal = cachedContract.contract.methods.proposeFundingRate(
{ rawValue: pricefeedPrice },
requestTimestamp
Expand Down Expand Up @@ -391,34 +411,14 @@ class FundingRateProposer {
};
}

// Load cached pricefeed and fetch current price. Returns null if failure to get either pricefeed or current price.
async _getPriceFeedAndPrice(fundingRateIdentifier) {
const priceFeed = this.priceFeedCache[fundingRateIdentifier];
if (!priceFeed) {
this.logger.error({
at: "PerpetualProposer",
message: "Failed to create pricefeed for funding rate identifier",
fundingRateIdentifier
});
return null;
}
let currentPrice = priceFeed.getCurrentPrice();
if (!currentPrice) {
this.logger.error({
at: "PerpetualProposer",
message: "Failed to query current price for funding rate identifier",
fundingRateIdentifier
});
return null;
}

// Round `offchainFundingRate` to desired number of decimals by converting back and forth between the pricefeed's
formatPriceToPricefeedPrecision(price, priceFeed) {
// Round `price` to desired number of decimals by converting back and forth between the pricefeed's
// configured precision:
return parseFixed(
// 1) `formatFixed` converts the price in wei to a floating point.
// 2) `toFixed` removes decimals beyond `this.precision` in the floating point.
// 3) `parseFixed` converts the floating point back into wei.
Number(formatFixed(currentPrice.toString(), priceFeed.getPriceFeedDecimals()))
Number(formatFixed(price.toString(), priceFeed.getPriceFeedDecimals()))
.toFixed(this.precision)
.toString(),
priceFeed.getPriceFeedDecimals()
Expand Down