properly handle pre swap fixed costs #173
Conversation
Costs such as the user wrapping ETH into WETH before a gouda swap should not be handled like other gas adjustments since they are _required_ to be paid by the user up-front. They should be factored into the startAmount and used in the decision making process between gouda and classic
marktoda
force-pushed
the
properly-handle-pre-swap-fixed-costs
branch
from
c207fe3
to
eb5669d
Compare
marktoda
force-pushed
the
properly-handle-pre-swap-fixed-costs
branch
from
eb5669d
to
4215d7e
Compare
lib/entities/quote/DutchQuote.ts
Outdated
| return DutchQuote.getGasAdjustedAmounts( | ||
| amounts, | ||
| // apply both the gouda gas adjustment and the routing gas adjustment | ||
| gasAdjustment.add(classicQuote.toJSON().gasUseEstimate), |
There was a problem hiding this comment.
Kind of confused here, so in this case we're passing in the GOUDA_BASE overhead, plus the original gas use estimation from routing-api into the gasAdjustment param. Isn't the gas from routing-api already factored into the amounts, thus this would be double counting it? I guess I may also be getting thrown off by the param names since imo gasAdjustment is strictly anything additional we are adding here
There was a problem hiding this comment.
I think we are using the non-gasAdjusted amounts from the routing API. So in that case we aren't double counting.
I believe we are only using the gasAdjustment param here to calculate a ratio of how much more/less quote token we get when we account for gouda gas overhead. See line 298.
cc: @marktoda at least I think so
There was a problem hiding this comment.
Oh that makes sense then if we are using quote
There was a problem hiding this comment.
we're actually using the gasAdjustment value.. i think eric is right; we're double counting :D
lib/entities/quote/DutchQuote.ts
Outdated
| return DutchQuote.getGasAdjustedAmounts( | ||
| amounts, | ||
| // apply both the gouda gas adjustment and the routing gas adjustment | ||
| gasAdjustment.add(classicQuote.toJSON().gasUseEstimate), |
There was a problem hiding this comment.
I think we are using the non-gasAdjusted amounts from the routing API. So in that case we aren't double counting.
I believe we are only using the gasAdjustment param here to calculate a ratio of how much more/less quote token we get when we account for gouda gas overhead. See line 298.
cc: @marktoda at least I think so
Pre-swap fixed costs like wrapping ETH should be factored into startAmounts instead of endAmounts
This commit also cleans up the gas adjustment logic to use gasNative / gasQuote from classic over a fixed number of gas units, rather than automatically including the original gas units.