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

refactor(scaledPriceAuthority): initialPrice by composition #7023

Merged
merged 2 commits into from
Feb 17, 2023

Conversation

dckc
Copy link
Member

@dckc dckc commented Feb 17, 2023

refs: #6957, #6960, #7008, #7016

Description

Rather than mucking with the internals of makePriceAuthorityTransform, compose it with a makeInitialTransform.

Security Considerations

more straightforward to audit

Documentation Considerations

makeInitialTransform has a bit of documentation as to its purpose and function.

Testing Considerations

Pure refactor; invisible to unit tests. :)

Tasks

Preview Give feedback

@dckc dckc requested review from turadg and michaelfig February 17, 2023 14:43
@dckc dckc force-pushed the dc-pa-initial branch 6 times, most recently from bb317cd to f1a6fe1 Compare February 17, 2023 14:56
packages/zoe/src/contracts/scaledPriceAuthority.js Outdated Show resolved Hide resolved
packages/zoe/src/contractSupport/priceAuthorityInitial.js Outdated Show resolved Hide resolved
Comment on lines 51 to 66
const quoteAmount = harden({
brand: quoteBrand,
value: [
{
amountIn,
amountOut,
timer,
timestamp,
},
],
});
const quotePayment = await E(quoteMint).mintPayment({
brand: quoteBrand,
value: [quoteAmount],
});
return harden({ quoteAmount, quotePayment });
Copy link
Member

Choose a reason for hiding this comment

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

dupes

const quoteAmount = {
brand: quoteBrand,
value: [{ amountIn, amountOut, timer, timestamp }],
};
const quotePayment = await E(quoteMint).mintPayment({
brand: quoteBrand,
value: [quoteAmount],
});
return harden({ quoteAmount, quotePayment });

consider DRYing. fine to have priceAuthorityInitial depend on priceAuthorityTransform so the latter could just export that maker function

Copy link
Member

Choose a reason for hiding this comment

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

LGTM, especially the mintQuote naming

});
return harden({ quoteAmount, quotePayment });
};
const initialUpdateP = oneQuote(
Copy link
Member

Choose a reason for hiding this comment

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

consider inlining this so the promises don't try to resolve if they're never needed

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'm pushing it down/in some, but I don't want to do a round trip to the timer service for clients that repeatedly call getUpdateSince() without bumping the updateCount.

refactor(scaledPriceAuthority): factor out mintQuote()

 - clarify priceOutPerIn
 - clarify mintCurrentQuote
@dckc dckc added the automerge:rebase Automatically rebase updates, then merge label Feb 17, 2023
@mergify mergify bot merged commit 0de845d into master Feb 17, 2023
@mergify mergify bot deleted the dc-pa-initial branch February 17, 2023 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants