Skip to content

Commit 2bef7ee

Browse files
danimhrdanimhr
authored
Add gas limit for internal calls in multicall (#100)
1 parent 831f437 commit 2bef7ee

15 files changed

+190
-53
lines changed

‎auction-server/.sqlx/query-96d94edc07a4bdffed54e9dc20b97551f25a428b19c285b91cad03be81cbc86c.json renamed to ‎auction-server/.sqlx/query-b8d0513ba6a00c8ad20b9c72677a69857e0f8ae8c2401c52f7f961c600574ffa.json

Lines changed: 4 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎auction-server/migrations/20240624171707_bid_gas_limit.down.sql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE bid DROP COLUMN gas_limit;

‎auction-server/migrations/20240624171707_bid_gas_limit.up.sql

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
ALTER TABLE bid ADD COLUMN gas_limit NUMERIC(78, 0);
2+
UPDATE bid SET gas_limit = 1000000000; -- indefinite gas limit
3+
ALTER TABLE bid ALTER COLUMN gas_limit SET NOT NULL;

‎auction-server/src/auction.rs

Lines changed: 76 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ use {
3333
contract::{
3434
abigen,
3535
ContractError,
36+
ContractRevert,
3637
EthError,
3738
EthEvent,
3839
FunctionCall,
@@ -107,17 +108,21 @@ pub type SignableProvider = TransformerMiddleware<
107108
>;
108109
pub type SignableExpressRelayContract = ExpressRelay<SignableProvider>;
109110

110-
impl From<([u8; 16], H160, Bytes, U256)> for MulticallData {
111-
fn from(x: ([u8; 16], H160, Bytes, U256)) -> Self {
111+
impl From<([u8; 16], H160, Bytes, U256, U256, bool)> for MulticallData {
112+
fn from(x: ([u8; 16], H160, Bytes, U256, U256, bool)) -> Self {
112113
MulticallData {
113-
bid_id: x.0,
114-
target_contract: x.1,
115-
target_calldata: x.2,
116-
bid_amount: x.3,
114+
bid_id: x.0,
115+
target_contract: x.1,
116+
target_calldata: x.2,
117+
bid_amount: x.3,
118+
gas_limit: x.4,
119+
revert_on_failure: x.5,
117120
}
118121
}
119122
}
120123

124+
const EXTRA_GAS_FOR_SUBMISSION: u32 = 500 * 1000;
125+
121126
pub fn get_simulation_call(
122127
relayer: Address,
123128
provider: Provider<TracedClient>,
@@ -157,26 +162,30 @@ impl Transformer for LegacyTxTransformer {
157162
pub async fn submit_bids(
158163
express_relay_contract: Arc<SignableExpressRelayContract>,
159164
permission: Bytes,
160-
multicall_data: Vec<MulticallData>,
165+
bids: Vec<SimulatedBid>,
161166
) -> Result<H256, ContractError<SignableProvider>> {
162-
let call = express_relay_contract.multicall(permission, multicall_data);
163-
let mut gas_estimate = call.estimate_gas().await?;
164-
165-
let gas_multiplier = U256::from(2); //TODO: smarter gas estimation
166-
gas_estimate *= gas_multiplier;
167-
let call_with_gas = call.gas(gas_estimate);
168-
let send_call = call_with_gas.send().await?;
169-
170-
Ok(send_call.tx_hash())
167+
let gas_estimate = bids.iter().fold(U256::zero(), |sum, b| sum + b.gas_limit);
168+
let tx_hash = express_relay_contract
169+
.multicall(
170+
permission,
171+
bids.into_iter().map(|b| (b, false).into()).collect(),
172+
)
173+
.gas(gas_estimate + EXTRA_GAS_FOR_SUBMISSION)
174+
.send()
175+
.await?
176+
.tx_hash();
177+
Ok(tx_hash)
171178
}
172179

173-
impl From<SimulatedBid> for MulticallData {
174-
fn from(bid: SimulatedBid) -> Self {
180+
impl From<(SimulatedBid, bool)> for MulticallData {
181+
fn from((bid, revert_on_failure): (SimulatedBid, bool)) -> Self {
175182
MulticallData {
176-
bid_id: bid.id.into_bytes(),
183+
bid_id: bid.id.into_bytes(),
177184
target_contract: bid.target_contract,
178185
target_calldata: bid.target_calldata,
179-
bid_amount: bid.bid_amount,
186+
bid_amount: bid.bid_amount,
187+
gas_limit: bid.gas_limit,
188+
revert_on_failure,
180189
}
181190
}
182191
}
@@ -208,7 +217,10 @@ async fn get_winner_bids(
208217
chain_store.provider.clone(),
209218
chain_store.config.clone(),
210219
permission_key.clone(),
211-
bids.clone().into_iter().map(|b| b.into()).collect(),
220+
bids.clone()
221+
.into_iter()
222+
.map(|b| (b, false).into())
223+
.collect(),
212224
)
213225
.await?;
214226

@@ -371,7 +383,7 @@ async fn submit_auction_for_bids<'a>(
371383
let submit_bids_call = submit_bids(
372384
chain_store.express_relay_contract.clone(),
373385
permission_key.clone(),
374-
winner_bids.clone().into_iter().map(|b| b.into()).collect(),
386+
winner_bids.clone(),
375387
);
376388

377389
match submit_bids_call.await {
@@ -612,9 +624,49 @@ pub async fn handle_bid(
612624
bid.target_contract,
613625
bid.target_calldata.clone(),
614626
bid.amount,
627+
U256::max_value(),
628+
// The gas estimation use some binary search algorithm to find the gas limit.
629+
// It reduce the upper bound threshold on success and increase the lower bound on revert.
630+
// If the contract does not reverts, the gas estimation will not be accurate in case of external call failures.
631+
// So we need to make sure in order to calculate the gas estimation correctly, the contract will revert if the external call fails.
632+
true,
615633
))],
616634
);
617635

636+
match call.clone().await {
637+
Ok(results) => {
638+
if !results[0].external_success {
639+
return Err(RestError::SimulationError {
640+
result: results[0].external_result.clone(),
641+
reason: results[0].multicall_revert_reason.clone(),
642+
});
643+
}
644+
}
645+
Err(e) => {
646+
return match e {
647+
ContractError::Revert(reason) => {
648+
if let Some(decoded_error) = ExpressRelayErrors::decode_with_selector(&reason) {
649+
if let ExpressRelayErrors::ExternalCallFailed(failure_result) =
650+
decoded_error
651+
{
652+
return Err(RestError::SimulationError {
653+
result: failure_result.status.external_result,
654+
reason: failure_result.status.multicall_revert_reason,
655+
});
656+
}
657+
}
658+
Err(RestError::BadParameters(format!(
659+
"Contract Revert Error: {}",
660+
reason,
661+
)))
662+
}
663+
ContractError::MiddlewareError { e: _ } => Err(RestError::TemporarilyUnavailable),
664+
ContractError::ProviderError { e: _ } => Err(RestError::TemporarilyUnavailable),
665+
_ => Err(RestError::BadParameters(format!("Error: {}", e))),
666+
};
667+
}
668+
}
669+
618670
let estimated_gas = call
619671
.estimate_gas()
620672
.await
@@ -639,29 +691,6 @@ pub async fn handle_bid(
639691
)
640692
.await?;
641693

642-
match call.await {
643-
Ok(results) => {
644-
if !results[0].external_success {
645-
return Err(RestError::SimulationError {
646-
result: results[0].external_result.clone(),
647-
reason: results[0].multicall_revert_reason.clone(),
648-
});
649-
}
650-
}
651-
Err(e) => {
652-
return match e {
653-
ContractError::Revert(reason) => Err(RestError::BadParameters(format!(
654-
"Contract Revert Error: {}",
655-
String::decode_with_selector(&reason)
656-
.unwrap_or("unable to decode revert".to_string())
657-
))),
658-
ContractError::MiddlewareError { e: _ } => Err(RestError::TemporarilyUnavailable),
659-
ContractError::ProviderError { e: _ } => Err(RestError::TemporarilyUnavailable),
660-
_ => Err(RestError::BadParameters(format!("Error: {}", e))),
661-
};
662-
}
663-
}
664-
665694
let bid_id = Uuid::new_v4();
666695
let simulated_bid = SimulatedBid {
667696
target_contract: bid.target_contract,
@@ -676,6 +705,8 @@ pub async fn handle_bid(
676705
Auth::Authorized(_, profile) => Some(profile.id),
677706
_ => None,
678707
},
708+
// Add a 25% more for estimation errors
709+
gas_limit: estimated_gas * U256::from(125) / U256::from(100),
679710
};
680711
store.add_bid(simulated_bid).await?;
681712
Ok(bid_id)

‎auction-server/src/models.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ pub struct Bid {
153153
pub bundle_index: BundleIndex,
154154
pub initiation_time: PrimitiveDateTime,
155155
pub profile_id: Option<ProfileId>,
156+
pub gas_limit: BigDecimal,
156157
}
157158

158159
impl Bid {

‎auction-server/src/opportunity_adapter.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,8 @@ pub async fn verify_opportunity(
185185
chain_store.config.adapter_factory_contract,
186186
adapter_calldata,
187187
fake_bid.amount,
188+
U256::max_value(),
189+
false,
188190
))],
189191
)
190192
.tx;

‎auction-server/src/state.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,10 @@ pub struct SimulatedBid {
111111
/// The profile id for the bid owner.
112112
#[schema(example = "", value_type = String)]
113113
pub profile_id: Option<models::ProfileId>,
114+
/// The gas limit for the contract call.
115+
#[schema(example = "2000000", value_type = String)]
116+
#[serde(with = "crate::serde::u256")]
117+
pub gas_limit: U256,
114118
}
115119

116120
pub type UnixTimestampMicros = i128;
@@ -359,6 +363,8 @@ impl TryFrom<(models::Bid, Option<models::Auction>)> for SimulatedBid {
359363
}
360364
let bid_amount = BidAmount::from_dec_str(bid.bid_amount.to_string().as_str())
361365
.map_err(|e| anyhow::anyhow!(e))?;
366+
let gas_limit = U256::from_dec_str(bid.gas_limit.to_string().as_str())
367+
.map_err(|e| anyhow::anyhow!(e))?;
362368
let bid_with_auction = (bid.clone(), auction);
363369
Ok(SimulatedBid {
364370
id: bid.id,
@@ -370,6 +376,7 @@ impl TryFrom<(models::Bid, Option<models::Auction>)> for SimulatedBid {
370376
status: bid_with_auction.try_into()?,
371377
initiation_time: bid.initiation_time.assume_offset(UtcOffset::UTC),
372378
profile_id: bid.profile_id,
379+
gas_limit,
373380
})
374381
}
375382
}
@@ -551,7 +558,7 @@ impl Store {
551558
pub async fn add_bid(&self, bid: SimulatedBid) -> Result<(), RestError> {
552559
let bid_id = bid.id;
553560
let now = OffsetDateTime::now_utc();
554-
sqlx::query!("INSERT INTO bid (id, creation_time, permission_key, chain_id, target_contract, target_calldata, bid_amount, status, initiation_time, profile_id) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)",
561+
sqlx::query!("INSERT INTO bid (id, creation_time, permission_key, chain_id, target_contract, target_calldata, bid_amount, status, initiation_time, profile_id, gas_limit) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11)",
555562
bid.id,
556563
PrimitiveDateTime::new(now.date(), now.time()),
557564
bid.permission_key.to_vec(),
@@ -562,6 +569,7 @@ impl Store {
562569
bid.status as _,
563570
PrimitiveDateTime::new(bid.initiation_time.date(), bid.initiation_time.time()),
564571
bid.profile_id,
572+
BigDecimal::from_str(&bid.gas_limit.to_string()).unwrap(),
565573
)
566574
.execute(&self.db)
567575
.await.map_err(|e| {

‎contracts/src/express-relay/Errors.sol

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// SPDX-License-Identifier: Apache 2
22
pragma solidity ^0.8.13;
33

4+
import "./Structs.sol";
5+
46
// Signature: 0x82b42900
57
error Unauthorized();
68

@@ -22,3 +24,6 @@ error DuplicateRelayerSubwallet();
2224

2325
// Signature: 0xac4d92b3
2426
error RelayerSubwalletNotFound();
27+
28+
// Signature: 0x740d0306
29+
error ExternalCallFailed(MulticallStatus status);

‎contracts/src/express-relay/ExpressRelay.sol

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,13 @@ contract ExpressRelay is Helpers, State, ExpressRelayEvents, ReentrancyGuard {
7777
multicallStatuses[i].multicallRevertReason = reason;
7878
}
7979

80+
if (
81+
!multicallStatuses[i].externalSuccess &&
82+
multicallData[i].revertOnFailure
83+
) {
84+
revert ExternalCallFailed(multicallStatuses[i]);
85+
}
86+
8087
// only count bid if call was successful (and bid was paid out)
8188
if (multicallStatuses[i].externalSuccess) {
8289
totalBid += multicallData[i].bidAmount;
@@ -142,7 +149,7 @@ contract ExpressRelay is Helpers, State, ExpressRelayEvents, ReentrancyGuard {
142149
(bool success, bytes memory result) = multicallData
143150
.targetContract
144151
.excessivelySafeCall(
145-
gasleft(), // this will automatically forward 63/64 of gas
152+
multicallData.gasLimit,
146153
0,
147154
32,
148155
multicallData.targetCalldata

‎contracts/src/express-relay/Structs.sol

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ struct MulticallData {
66
address targetContract;
77
bytes targetCalldata;
88
uint256 bidAmount;
9+
uint256 gasLimit;
10+
bool revertOnFailure;
911
}
1012

1113
struct MulticallStatus {

0 commit comments

Comments
 (0)