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 Quotecall #191

Merged
merged 4 commits into from
May 1, 2024
Merged

Add Quotecall #191

merged 4 commits into from
May 1, 2024

Conversation

hayesgm
Copy link
Contributor

@hayesgm hayesgm commented Mar 23, 2024

Quotecall is like Paycall, except it always sends a specific quote of USDC to tx.origin allowing for minor changes in gas price (i.e. via a maxDeltaPercentage parameter). As the USDC is sent before the sub-transaction is run, this allow things like "send max USDC" to properly work as expected. As the quote is fixed, this means that recurring or delayed transactions are very unlikely to work with this script.

Copy link
Collaborator

@kevincheng96 kevincheng96 left a comment

Choose a reason for hiding this comment

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

The contract logic looks sound to me, though this PR is missing unit tests.

src/quark-core-scripts/src/Quotecall.sol Outdated Show resolved Hide resolved
src/quark-core-scripts/src/Quotecall.sol Outdated Show resolved Hide resolved
hayesgm and others added 2 commits April 30, 2024 10:18
Quotecall is like Paycall, except it always sends a specific quote of USDC to `tx.origin` allowing for minor changes in gas price (i.e. via a `deltaBps` parameter). As the USDC is sent _before_ the sub-transaction is run, this allow things like "send max USDC" to properly work as expected. As the quote is fixed, this means that recurring or delayed transactions are very _unlikely_ to work with this script.
@kevincheng96
Copy link
Collaborator

I took over the branch and made the following improvements/changes:

  • Add PayForGas event
  • Add propogateReverts flag in constructor
  • Change maxDeltaBps to maxDeltaPercentage and explicitly define PERCENTAGE_SCALE as 1e18 (e.g. 1e18 = 100%)
  • Add Chainlink price checks to make sure prices returned are > 0
  • Rename ethBasedPriceFeedAddress to nativeTokenBasedPriceFeedAddress since some chains don't use ETH as the native token
  • Add unit tests

Copy link
Collaborator

@kevincheng96 kevincheng96 left a comment

Choose a reason for hiding this comment

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

Geoff reviewed my changes and gave the LGTM. I'll go ahead and approve and merge since this is his PR and he can't give the approval.

@kevincheng96 kevincheng96 merged commit 9b334b0 into main May 1, 2024
4 checks passed
@kevincheng96 kevincheng96 deleted the hayesgm/quotecall branch May 1, 2024 17:21
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.

2 participants