Skip to content

Commit

Permalink
fix: Gas Policy Minimum Handling to Prevent Unexpected Behavior (hype…
Browse files Browse the repository at this point in the history 
…rlane-xyz#5517)

### Description

Prior to this change, configuring the `gaspaymentenforcement` parameter
in the config to a minimum of 0 resulted in unexpected behavior. With
this configuration, the relayer would relay any message from the
designated relay chains, whether or not the message paid the specified
IGP, resulting in the same behavior as `gaspaymentenforcement` of None.

This is due to `fn message_meets_gas_payment_requirement` in
`rust/main/agents/relayer/src/msg/gas_payment/mod.rs`.

```rs
        let current_payment_option = self
            .db
            .retrieve_gas_payment_by_gas_payment_key(gas_payment_key)?;

        let current_payment = match current_payment_option {
            Some(payment) => payment,
            None => InterchainGasPayment::from_gas_payment_key(gas_payment_key),
        };
        
```

If the payment does not exist (because it was not indexed on the
specified IGP), `current_payment` is set to
`InterchainGasPayment::from_gas_payment_key(gas_payment_key)` which
creates an InterchainGasPayment with a default payment value of 0. As a
result, payments to *other* IGPs appeared to be 0 payments to the
specified IGP. Therefore, in the loop later in the function, the message
matches with `GasPolicyStatus::PolicyMet(gas_limit)` and the relayer
relays it.

```rs
            return policy
                .message_meets_gas_payment_requirement(
                    message,
                    &current_payment,
                    &current_expenditure,
                    tx_cost_estimate,
                )
                .await
                .map(|result| {
                    if let Some(gas_limit) = result {
                        GasPolicyStatus::PolicyMet(gas_limit)
                    } else if current_payment_option.is_some() {
                        // There is a gas payment but it didn't meet the policy
                        GasPolicyStatus::PolicyNotMet
                    } else {
                        // No payment was found and it didn't meet the policy
                        GasPolicyStatus::NoPaymentFound
                    }
                });
 ```
These changes create the desired behavior, namely, that setting `gaspaymentenforcement` to a minimum of 0 should only relay messages that hooked / emitted from the specified IGP. 

### Changes

The desired functionality is implemented by modifying the `message_meets_gas_payment_requirement` function in `rust/main/agents/relayer/src/msg/gas_payment/mod.rs`. The change adds a check in that function to ensure that the `self.db.retrieve_gas_payment_by_gas_payment_key(gas_payment_key)?;` did in fact return a payment on the IGP from the database. 

```rs
            if policy.requires_payment_found() && !payment_found {
                return Ok(GasPolicyStatus::NoPaymentFound);
            }
```
The check also calls a new function added to the `GasPaymentPolicy` trait called `requires_payment_found`. By default, this is set to return `false`, which allows the `none.rs` gas policy to skip this logic and continue relaying all messages. For the `minimum.rs` and `on_chain_fee_quoting.rs` gas policies, this is set to return `true` which requires a gas payment event to have been found on the specified IGP to continue. 

### Related issues

N/A

### Backward compatibility

Unsure, as I am not as familiar with the history associated with this codebase. 

### Testing

Added unit tests and am currently running manual tests to ensure only messages that were emitted from the specified IGP were relayed.
  • Loading branch information
@christianangelopoulos
christianangelopoulos authored Feb 28, 2025
1 parent e861535 commit 9fc5e66
Show file tree
Hide file tree
Showing 4 changed files with 319 additions and 3 deletions.
292 changes: 291 additions & 1 deletion rust/main/agents/relayer/src/msg/gas_payment/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ pub trait GasPaymentPolicy: Debug + Send + Sync {
current_expenditure: &InterchainGasExpenditure,
tx_cost_estimate: &TxCostEstimate,
) -> Result<Option<U256>>;

fn requires_payment_found(&self) -> bool {
false
}
}

#[derive(PartialEq, Debug)]
Expand Down Expand Up @@ -96,6 +100,9 @@ impl GasPaymentEnforcer {
let current_payment_option = self
.db
.retrieve_gas_payment_by_gas_payment_key(gas_payment_key)?;

let payment_found = current_payment_option.is_some();

let current_payment = match current_payment_option {
Some(payment) => payment,
None => InterchainGasPayment::from_gas_payment_key(gas_payment_key),
Expand Down Expand Up @@ -127,6 +134,11 @@ impl GasPaymentEnforcer {
?tx_cost_estimate,
"Evaluating if message meets gas payment requirement",
);

if policy.requires_payment_found() && !payment_found {
return Ok(GasPolicyStatus::NoPaymentFound);
}

return policy
.message_meets_gas_payment_requirement(
message,
Expand All @@ -138,7 +150,7 @@ impl GasPaymentEnforcer {
.map(|result| {
if let Some(gas_limit) = result {
GasPolicyStatus::PolicyMet(gas_limit)
} else if current_payment_option.is_some() {
} else if payment_found {
// There is a gas payment but it didn't meet the policy
GasPolicyStatus::PolicyNotMet
} else {
Expand Down Expand Up @@ -445,4 +457,282 @@ mod test {
})
.await;
}

#[tokio::test]
async fn test_no_payment_found_minimum_policy() {
#[allow(unused_must_use)]
test_utils::run_test_db(|db| async move {
let hyperlane_db = HyperlaneRocksDB::new(
&HyperlaneDomain::new_test_domain("test_no_payment_found_minimum_policy"),
db,
);

let enforcer = GasPaymentEnforcer::new(
// Require a payment
vec![GasPaymentEnforcementConf {
policy: GasPaymentEnforcementPolicy::Minimum {
payment: U256::from(0),
},
matching_list: MatchingList::default(),
}],
hyperlane_db,
);

assert!(matches!(
enforcer
.message_meets_gas_payment_requirement(
&HyperlaneMessage::default(),
&TxCostEstimate::default(),
)
.await,
Ok(GasPolicyStatus::NoPaymentFound)
));
})
.await;
}

#[tokio::test]
async fn test_deminimis_payment_found_minimum_policy() {
#[allow(unused_must_use)]
test_utils::run_test_db(|db| async move {
let hyperlane_db = HyperlaneRocksDB::new(
&HyperlaneDomain::new_test_domain("test_deminimis_payment_found_minimum_policy"),
db,
);

let payment = InterchainGasPayment {
message_id: HyperlaneMessage::default().id(),
destination: HyperlaneMessage::default().destination,
payment: U256::from(1),
gas_amount: U256::from(1),
};
hyperlane_db.process_gas_payment(payment, &LogMeta::random());

let enforcer = GasPaymentEnforcer::new(
// Require a payment
vec![GasPaymentEnforcementConf {
policy: GasPaymentEnforcementPolicy::Minimum {
payment: U256::from(1),
},
matching_list: MatchingList::default(),
}],
hyperlane_db,
);

assert_eq!(
enforcer
.message_meets_gas_payment_requirement(
&HyperlaneMessage::default(),
&TxCostEstimate {
gas_limit: U256::one(),
..Default::default()
},
)
.await
.unwrap(),
GasPolicyStatus::PolicyMet(U256::one())
);
})
.await;
}

#[tokio::test]
async fn test_zero_payment_found_minimum_policy() {
#[allow(unused_must_use)]
test_utils::run_test_db(|db| async move {
let hyperlane_db = HyperlaneRocksDB::new(
&HyperlaneDomain::new_test_domain("test_zero_payment_found_miniumn_policy"),
db,
);

let payment = InterchainGasPayment {
message_id: HyperlaneMessage::default().id(),
destination: HyperlaneMessage::default().destination,
payment: U256::from(0),
gas_amount: U256::from(0),
};
hyperlane_db.process_gas_payment(payment, &LogMeta::random());

let enforcer = GasPaymentEnforcer::new(
// Require a payment
vec![GasPaymentEnforcementConf {
policy: GasPaymentEnforcementPolicy::Minimum {
payment: U256::from(0),
},
matching_list: MatchingList::default(),
}],
hyperlane_db,
);

assert_eq!(
enforcer
.message_meets_gas_payment_requirement(
&HyperlaneMessage::default(),
&TxCostEstimate {
gas_limit: U256::one(),
..Default::default()
},
)
.await
.unwrap(),
GasPolicyStatus::PolicyMet(U256::one())
);
})
.await;
}

#[tokio::test]
async fn test_no_payment_found_none_policy() {
#[allow(unused_must_use)]
test_utils::run_test_db(|db| async move {
let hyperlane_db = HyperlaneRocksDB::new(
&HyperlaneDomain::new_test_domain("test_no_payment_found_none_policy"),
db,
);

let enforcer = GasPaymentEnforcer::new(
// Require a payment
vec![GasPaymentEnforcementConf {
policy: GasPaymentEnforcementPolicy::None,
matching_list: MatchingList::default(),
}],
hyperlane_db,
);

assert_eq!(
enforcer
.message_meets_gas_payment_requirement(
&HyperlaneMessage::default(),
&TxCostEstimate::default(),
)
.await
.unwrap(),
GasPolicyStatus::PolicyMet(U256::zero())
);
})
.await;
}

#[tokio::test]
async fn test_payment_found_none_policy() {
#[allow(unused_must_use)]
test_utils::run_test_db(|db| async move {
let hyperlane_db = HyperlaneRocksDB::new(
&HyperlaneDomain::new_test_domain("test_payment_found_none_policy"),
db,
);

let payment = InterchainGasPayment {
message_id: HyperlaneMessage::default().id(),
destination: HyperlaneMessage::default().destination,
payment: U256::from(1),
gas_amount: U256::from(1),
};
hyperlane_db.process_gas_payment(payment, &LogMeta::random());

let enforcer = GasPaymentEnforcer::new(
// Require a payment
vec![GasPaymentEnforcementConf {
policy: GasPaymentEnforcementPolicy::None,
matching_list: MatchingList::default(),
}],
hyperlane_db,
);

assert_eq!(
enforcer
.message_meets_gas_payment_requirement(
&HyperlaneMessage::default(),
&TxCostEstimate {
gas_limit: U256::one(),
..Default::default()
},
)
.await
.unwrap(),
GasPolicyStatus::PolicyMet(U256::one())
);
})
.await;
}

#[tokio::test]
async fn test_no_payment_found_quote_policy() {
#[allow(unused_must_use)]
test_utils::run_test_db(|db| async move {
let hyperlane_db = HyperlaneRocksDB::new(
&HyperlaneDomain::new_test_domain("test_no_payment_found_quote_policy"),
db,
);

let enforcer = GasPaymentEnforcer::new(
// Require a payment
vec![GasPaymentEnforcementConf {
policy: GasPaymentEnforcementPolicy::OnChainFeeQuoting {
gas_fraction_numerator: 1,
gas_fraction_denominator: 2,
},
matching_list: MatchingList::default(),
}],
hyperlane_db,
);

assert!(matches!(
enforcer
.message_meets_gas_payment_requirement(
&HyperlaneMessage::default(),
&TxCostEstimate::default(),
)
.await,
Ok(GasPolicyStatus::NoPaymentFound)
));
})
.await;
}

#[tokio::test]
async fn test_payment_found_quote_policy() {
#[allow(unused_must_use)]
test_utils::run_test_db(|db| async move {
let hyperlane_db = HyperlaneRocksDB::new(
&HyperlaneDomain::new_test_domain("test_payment_found_quote_policy"),
db,
);

let payment = InterchainGasPayment {
message_id: HyperlaneMessage::default().id(),
destination: HyperlaneMessage::default().destination,
payment: U256::from(1),
gas_amount: U256::from(50),
};
hyperlane_db.process_gas_payment(payment, &LogMeta::random());

let enforcer = GasPaymentEnforcer::new(
// Require a payment
vec![GasPaymentEnforcementConf {
policy: GasPaymentEnforcementPolicy::OnChainFeeQuoting {
gas_fraction_numerator: 1,
gas_fraction_denominator: 2,
},
matching_list: MatchingList::default(),
}],
hyperlane_db,
);

assert_eq!(
enforcer
.message_meets_gas_payment_requirement(
&HyperlaneMessage::default(),
&TxCostEstimate {
gas_limit: U256::from(100),
..Default::default()
},
)
.await
.unwrap(),
GasPolicyStatus::PolicyMet(U256::from(100))
);
})
.await;
}
}
11 changes: 11 additions & 0 deletions rust/main/agents/relayer/src/msg/gas_payment/policies/minimum.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ pub struct GasPaymentPolicyMinimum {

#[async_trait]
impl GasPaymentPolicy for GasPaymentPolicyMinimum {
/// `Minimum` requires a payment to exist on the IGP specified in the config,
/// even if the payment is zero. For example, a policy of Minimum { payment: 0 }
/// will only relay messages that send a zero payment to the IGP specified in the config.
/// This is different from not requiring message senders to make any payment at all to
/// the configured IGP to get relayed. To relay regardless of the existence of a payment,
/// the `None` IGP policy should be used.
async fn message_meets_gas_payment_requirement(
&self,
_message: &HyperlaneMessage,
Expand All @@ -28,6 +35,10 @@ impl GasPaymentPolicy for GasPaymentPolicyMinimum {
Ok(None)
}
}

fn requires_payment_found(&self) -> bool {
true
}
}

#[tokio::test]
Expand Down
8 changes: 8 additions & 0 deletions rust/main/agents/relayer/src/msg/gas_payment/policies/on_chain_fee_quoting.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ impl Default for GasPaymentPolicyOnChainFeeQuoting {

#[async_trait]
impl GasPaymentPolicy for GasPaymentPolicyOnChainFeeQuoting {
/// OnChainFeeQuoting requires the user to pay a specified fraction of the
/// estimated gas. Like the Minimum policy, OnChainFeeQuoting requires a
/// payment to exist on the IGP specified in the config.
async fn message_meets_gas_payment_requirement(
&self,
_message: &HyperlaneMessage,
Expand All @@ -59,6 +63,10 @@ impl GasPaymentPolicy for GasPaymentPolicyOnChainFeeQuoting {
Ok(None)
}
}

fn requires_payment_found(&self) -> bool {
true
}
}

#[cfg(test)]
Expand Down
11 changes: 9 additions & 2 deletions rust/main/agents/relayer/src/settings/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,19 @@ pub struct GasPaymentEnforcementConf {
#[derive(Debug, Clone, Default)]
pub enum GasPaymentEnforcementPolicy {
/// No requirement - all messages are processed regardless of gas payment
/// and regardless of whether a payment for the message was processed by the specified IGP.
#[default]
None,
/// Messages that have paid a minimum amount will be processed
/// `Minimum` requires a payment to exist on the IGP specified in the config,
/// even if the payment is zero. For example, a policy of Minimum { payment: 0 }
/// will only relay messages that send a zero payment to the IGP specified in the config.
/// This is different from not requiring message senders to make any payment at all to
/// the configured IGP to get relayed. To relay regardless of the existence of a payment,
/// the `None` IGP policy should be used.
Minimum { payment: U256 },
/// The required amount of gas on the foreign chain has been paid according
/// to on-chain fee quoting.
/// to on-chain fee quoting. OnChainFeeQuoting requires a payment to exist
/// on the IGP specified in the config.
OnChainFeeQuoting {
gas_fraction_numerator: u64,
gas_fraction_denominator: u64,
Expand Down

0 comments on commit 9fc5e66

Please sign in to comment.