Skip to content

Commit

Permalink
[Easy] Pass settlement contract as from address when requesting 1Inch… (
Browse files Browse the repository at this point in the history
#1817)

Otherwise 1Inch may suggest routes in which the proceeds are transferred
directly to the trader (instead of the settlement contract), which will
make the settlement call fail. Cf this
[simulation](https://dashboard.tenderly.co/gp-v2/barn/simulator/0c112512-c0be-4675-a06f-ab0c153bd4d6)
for instance.

## Test Plan

The integration test can't be easily get working because 1Inch closed
down their public API and their dev API uses a different format (using
our endpoint would also require injecting custom header). We use the
settlement contract already when requesting solutions though (cf
[here](https://github.com/cowprotocol/services/blob/ea860194f4e16e97680490660ebc494909959df7/crates/solver/src/solver/oneinch_solver.rs#L116)).
  • Loading branch information
fleupold authored Aug 29, 2023
1 parent a98a724 commit d467474
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 8 deletions.
1 change: 1 addition & 0 deletions crates/shared/src/price_estimation/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,7 @@ impl PriceEstimatorCreating for OneInchPriceEstimator {
factory.rate_limiter(name),
factory.shared_args.one_inch_referrer_address,
solver,
factory.network.settlement,
))
}

Expand Down
3 changes: 3 additions & 0 deletions crates/shared/src/price_estimation/oneinch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@ impl OneInchPriceEstimator {
rate_limiter: Arc<RateLimiter>,
referrer_address: Option<H160>,
solver: H160,
settlement_contract: H160,
) -> Self {
Self(TradeEstimator::new(
Arc::new(OneInchTradeFinder::new(
api,
disabled_protocols,
referrer_address,
solver,
settlement_contract,
)),
rate_limiter,
"oneinch".into(),
Expand Down Expand Up @@ -74,6 +76,7 @@ mod tests {
)),
None,
H160([1; 20]),
H160([2; 20]),
)
}

Expand Down
35 changes: 27 additions & 8 deletions crates/shared/src/trade_finding/oneinch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ struct Inner {
cache: Cache,
referrer_address: Option<H160>,
solver: H160,
settlement_contract: H160,
}

#[derive(Clone, Eq, PartialEq)]
Expand All @@ -46,13 +47,15 @@ impl OneInchTradeFinder {
disabled_protocols: Vec<String>,
referrer_address: Option<H160>,
solver: H160,
settlement_contract: H160,
) -> Self {
Self {
inner: Arc::new(Inner::new(
api,
disabled_protocols,
referrer_address,
solver,
settlement_contract,
)),
sharing: RequestSharing::labelled("oneinch".into()),
}
Expand Down Expand Up @@ -109,13 +112,15 @@ impl Inner {
disabled_protocols: Vec<String>,
referrer_address: Option<H160>,
solver: H160,
settlement_contract: H160,
) -> Self {
Self {
api,
disabled_protocols,
referrer_address,
cache: Default::default(),
solver,
settlement_contract,
}
}

Expand Down Expand Up @@ -171,11 +176,7 @@ impl Inner {
query.sell_token,
query.buy_token,
query.in_amount,
query
.verification
.as_ref()
.map(|v| v.from)
.unwrap_or_default(),
self.settlement_contract,
allowed_protocols,
Slippage::ONE_PERCENT,
self.referrer_address,
Expand Down Expand Up @@ -225,7 +226,13 @@ mod tests {
};

fn create_trade_finder<T: OneInchClient>(api: T) -> OneInchTradeFinder {
OneInchTradeFinder::new(Arc::new(api), Vec::default(), None, H160([1; 20]))
OneInchTradeFinder::new(
Arc::new(api),
Vec::default(),
None,
H160([1; 20]),
H160([2; 20]),
)
}

#[tokio::test]
Expand Down Expand Up @@ -490,7 +497,13 @@ mod tests {
.expect_get_swap()
.return_once(|_| async { Ok(Default::default()) }.boxed());

let trader = OneInchTradeFinder::new(Arc::new(oneinch), Vec::new(), None, H160([1; 20]));
let trader = OneInchTradeFinder::new(
Arc::new(oneinch),
Vec::new(),
None,
H160([1; 20]),
H160([1; 20]),
);

let query = Query {
kind: OrderKind::Sell,
Expand Down Expand Up @@ -546,7 +559,13 @@ mod tests {

let mut inner = Inner {
cache: Cache::new(MAX_AGE),
..Inner::new(Arc::new(mock_api(1)), vec![], None, H160([1; 20]))
..Inner::new(
Arc::new(mock_api(1)),
vec![],
None,
H160([1; 20]),
H160([1; 20]),
)
};

// Calling `Inner::spender()` twice within `MAX_AGE` will return
Expand Down

0 comments on commit d467474

Please sign in to comment.