fix: Gas Policy Minimum Handling to Prevent Unexpected Behavior #5517

christianangelopoulos · 2025-02-19T07:09:17Z

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.

        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.

            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.

            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.

changeset-bot · 2025-02-19T07:09:21Z

⚠️ No Changeset found

Latest commit: 89a8e7e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

shahrukhtrao · 2025-02-19T11:36:37Z

@christianangelopoulos put fix: as prefix to pass linter

christianangelopoulos · 2025-02-20T18:57:52Z

added comments clarifying the requirements of each gas policy per request from @daniel-savu

daniel-savu

looks great, thank you for taking the time to look into this! Left mostly nits

daniel-savu · 2025-02-21T12:03:58Z

rust/main/agents/relayer/src/msg/gas_payment/mod.rs

+                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),


should we test for the zero payment edge case here since that's the goal of this PR. So with these params and some changes below as well, and the policy should still be met

Suggested change

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),

payment: U256::zero(),

gas_amount: U256::zero(),

};

hyperlane_db.process_gas_payment(payment, &LogMeta::random());

let enforcer = GasPaymentEnforcer::new(

// Require a payment

vec![GasPaymentEnforcementConf {

policy: GasPaymentEnforcementPolicy::Minimum {

payment: U256::zero(),

daniel-savu · 2025-02-21T12:17:07Z

rust/main/agents/relayer/src/msg/gas_payment/mod.rs

+            let payment = InterchainGasPayment {
+                message_id: HyperlaneMessage::default().id(),
+                destination: HyperlaneMessage::default().destination,
+                payment: U256::from(100),


this can be anything so I'd set it to like 1 to make it easier to understand it's not related to the 100 in TxCostEstimate

daniel-savu · 2025-02-21T12:17:54Z

rust/main/agents/relayer/src/msg/gas_payment/mod.rs

+                message_id: HyperlaneMessage::default().id(),
+                destination: HyperlaneMessage::default().destination,
+                payment: U256::from(100),
+                gas_amount: U256::from(100),


since the test uses a numerator of 1 and a denominator of 2 let's set this to 50 to test the edge case

Suggested change

gas_amount: U256::from(100),

gas_amount: U256::from(50),

daniel-savu · 2025-02-21T12:23:25Z

rust/main/agents/relayer/src/msg/gas_payment/policies/minimum.rs

+    /// Minimum requires a message 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 are processed by the IGP specified in the config.
+    /// If you want to relay all messages regardless of payment and IGP, use `None`.


nit

Suggested change

/// Minimum requires a message 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 are processed by the IGP specified in the config.

/// If you want to relay all messages regardless of payment and IGP, use `None`.

/// `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.

daniel-savu · 2025-02-21T12:25:57Z

rust/main/agents/relayer/src/msg/gas_payment/policies/on_chain_fee_quoting.rs

+
+    /// OnChainFeeQuoting requires the user to pay a specified fraction of the 
+    /// estimated gas. Like the Minimum policy, OnChainFeeQuoting requires a 
+    /// message to exist on the IGP specified in the config.


nit

Suggested change

/// message to exist on the IGP specified in the config.

/// payment to exist on the IGP specified in the config.

daniel-savu · 2025-02-21T12:26:39Z

rust/main/agents/relayer/src/settings/mod.rs

@@ -81,12 +81,17 @@ pub struct GasPaymentEnforcementConf {
 #[derive(Debug, Clone, Default)]
 pub enum GasPaymentEnforcementPolicy {
    /// No requirement - all messages are processed regardless of gas payment
+    /// and regardless of whether the message was processed by the specified IGP.


nit

Suggested change

/// and regardless of whether the message was processed by the specified IGP.

/// and regardless of whether a payment for the message was processed by the specified IGP.

daniel-savu · 2025-02-21T12:26:59Z

rust/main/agents/relayer/src/settings/mod.rs

+    /// even if the payment is zero. For example, a policy of Minimum { payment: 0 } 
+    /// will only relay messages that are processed by the IGP specified in the config.


ditto as for message_meets_gas_payment_requirement

christianangelopoulos added 2 commits February 19, 2025 00:09

added logic to ensure no delivery of messages not sent to IGP

11068ab

small cleanup

213e951

christianangelopoulos requested review from tkporter and daniel-savu as code owners February 19, 2025 07:09

christianangelopoulos changed the title ~~Fix Gas Policy Minimum Handling to Prevent Unexpected Behavior~~ fix: Gas Policy Minimum Handling to Prevent Unexpected Behavior Feb 19, 2025

added comments to clarify gas policy requirements

89a8e7e

daniel-savu reviewed Feb 21, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Gas Policy Minimum Handling to Prevent Unexpected Behavior #5517

fix: Gas Policy Minimum Handling to Prevent Unexpected Behavior #5517

christianangelopoulos commented Feb 19, 2025

changeset-bot bot commented Feb 19, 2025 •

edited

Loading

shahrukhtrao commented Feb 19, 2025 •

edited

Loading

christianangelopoulos commented Feb 20, 2025

daniel-savu left a comment

daniel-savu Feb 21, 2025

daniel-savu Feb 21, 2025

daniel-savu Feb 21, 2025

daniel-savu Feb 21, 2025

daniel-savu Feb 21, 2025

daniel-savu Feb 21, 2025

daniel-savu Feb 21, 2025

	/// message to exist on the IGP specified in the config.
	/// payment to exist on the IGP specified in the config.

	/// and regardless of whether the message was processed by the specified IGP.
	/// and regardless of whether a payment for the message was processed by the specified IGP.

		/// even if the payment is zero. For example, a policy of Minimum { payment: 0 }
		/// will only relay messages that are processed by the IGP specified in the config.

fix: Gas Policy Minimum Handling to Prevent Unexpected Behavior #5517

Are you sure you want to change the base?

fix: Gas Policy Minimum Handling to Prevent Unexpected Behavior #5517

Conversation

christianangelopoulos commented Feb 19, 2025

Description

Changes

Related issues

Backward compatibility

Testing

changeset-bot bot commented Feb 19, 2025 • edited Loading

⚠️ No Changeset found

shahrukhtrao commented Feb 19, 2025 • edited Loading

christianangelopoulos commented Feb 20, 2025

daniel-savu left a comment

Choose a reason for hiding this comment

daniel-savu Feb 21, 2025

Choose a reason for hiding this comment

daniel-savu Feb 21, 2025

Choose a reason for hiding this comment

daniel-savu Feb 21, 2025

Choose a reason for hiding this comment

daniel-savu Feb 21, 2025

Choose a reason for hiding this comment

daniel-savu Feb 21, 2025

Choose a reason for hiding this comment

daniel-savu Feb 21, 2025

Choose a reason for hiding this comment

daniel-savu Feb 21, 2025

Choose a reason for hiding this comment

changeset-bot bot commented Feb 19, 2025 •

edited

Loading

shahrukhtrao commented Feb 19, 2025 •

edited

Loading