-
Notifications
You must be signed in to change notification settings - Fork 93
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
Solver crate forwards flashloans back to driver #3283
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,6 +34,7 @@ impl Auction { | |
fee_handler: FeeHandler, | ||
solver_native_token: ManageNativeToken, | ||
flashloans_enabled: bool, | ||
flashloan_default_lender: eth::Address, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By now this constructor has quite a few arguments that will not change during the runtime of the driver. |
||
) -> Self { | ||
let mut tokens: HashMap<eth::H160, _> = auction | ||
.tokens() | ||
|
@@ -161,7 +162,14 @@ impl Auction { | |
), | ||
app_data: AppDataHash(order.app_data.hash().0.into()), | ||
flashloan_hint: flashloans_enabled | ||
.then(|| order.app_data.flashloan().cloned().map(Into::into)) | ||
.then(|| { | ||
order.app_data.flashloan().map(|flashloan| FlashloanHint { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if filling in these default values makes more sense in the component that fetches the app data in the first place. 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True. For now, flashloan hints in their final shape are not needed/used in the driver domain. But we can move this logic out to driver domain when the need comes. |
||
lender: flashloan.lender.unwrap_or(flashloan_default_lender.0), | ||
borrower: flashloan.borrower.unwrap_or(order.uid.owner().0), | ||
token: flashloan.token, | ||
amount: flashloan.amount, | ||
}) | ||
}) | ||
.flatten(), | ||
signature: order.signature.data.clone().into(), | ||
signing_scheme: match order.signature.scheme { | ||
|
@@ -609,24 +617,13 @@ struct ForeignLimitOrder { | |
#[derive(Debug, Serialize)] | ||
#[serde(rename_all = "camelCase")] | ||
pub struct FlashloanHint { | ||
pub lender: Option<eth::H160>, | ||
pub borrower: Option<eth::H160>, | ||
pub lender: eth::H160, | ||
pub borrower: eth::H160, | ||
pub token: eth::H160, | ||
#[serde_as(as = "serialize::U256")] | ||
pub amount: eth::U256, | ||
} | ||
|
||
impl From<app_data::Flashloan> for FlashloanHint { | ||
fn from(value: app_data::Flashloan) -> Self { | ||
Self { | ||
lender: value.lender, | ||
borrower: value.borrower, | ||
token: value.token, | ||
amount: value.amount, | ||
} | ||
} | ||
} | ||
|
||
fn fee_to_decimal(fee: liquidity::balancer::v2::Fee) -> bigdecimal::BigDecimal { | ||
bigdecimal::BigDecimal::new(fee.as_raw().to_big_int(), 18) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. I wasn't aware that the contract @fedgiac used in the e2e test for the contract only worked with Dai. I'm fine to make this the default for the start where it's only relevant for the e2e test but we should have at least 1 supported general purpose flashloan lender.
Also this could indicate that we might need a module or component that suggests a good lending provider based on the token that needs to be borrowed. Should happen later, though (if at all).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree. There are A LOT of assumptions made at the moment, but I have them in my list. Before going live, these will have to be addressed and some kind of validation for flashloan hints will be implemented to avoid weird cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In principle there are wrappers for incompatible flash-loan lenders (some examples here from this EIP discussion).
But for the e2e test I wanted to avoid the extra mental overhead of the wrapper and I used a protocol that natively supports the standard (but unfortunately only for DAI).
Another assumption in the e2e test is that the flash-loan lender pulls back the token from the borrower by executing
transferFrom
, but this isn't necessarily the case for other protocols, some other expect the tokens to be transferred back and check that the balance at the end is as expected.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we will have to implement a lender specific encoding once we finish this first version that works only with Sky.