-
Notifications
You must be signed in to change notification settings - Fork 1
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
On chain fee quoting #3
base: main
Are you sure you want to change the base?
Conversation
HIP-3.md
Outdated
bytes32 indexed messageId, | ||
uint256 amount, | ||
uint256 gas, | ||
IsmType ismType |
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.
IMO this complexity is not worth it. I'd advocate for a simpler interface where the user just specifies how much gas they want on the remote chain. They need to understand that for "handle" anyways, I'm not sure it's more work to incorporate the gas costs of message verification into that
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.
Curious how you feel about the Default vs non-default ISM distinction
section down below then
I really want to avoid having users default to an ISM that's out of their control and then they're responsible for being aware of changes to the ISM
Maybe there could be a specific InterchainGasPaymaster that's for default ISM consumers, and a separate one that's for non-default ISM consumers?
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.
e.g. imagine we have a quorum of 5 today on the default multisig ISM, people hardcode gas costs based off this, and now our hands are tied in ever increasing that quorum
HIP-3.md
Outdated
* ISM.verify gas, and gas required by the recipient's `handle` function. | ||
* @param _refundAddress The address to refund any msg.value overpayment to. | ||
*/ | ||
function payGasForNonDefaultIsm( |
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.
What about just payGas
HIP-3.md
Outdated
bytes32 indexed messageId, | ||
uint256 amount, | ||
uint256 gas, | ||
IsmType ismType |
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.
Why is this parameter neccessary?
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.
as it's written, there's slightly different relayer behavior based off the ism type
HIP-3.md
Outdated
|
||
The off-chain relayer will index `GasPayment` events and relate them to dispatched messages based off their message ID. | ||
|
||
For the default ISM case, the relayer will create the process transaction with a gas limit that results in at least `_handleGas` gas able to be used by the recipient’s `handle` function. The total gas limit of the transaction can be found by the relayer estimating all other gas costs (i.e. intrinsic gas, calldata gas including the ISM metadata and message body, ISM.verify gas, and Mailbox overhead gas) and adding this to the `_handleGas`. |
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.
Doesn't the IGP will need to calculate all of this also?
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.
Yeah so the idea is to have that be read from storage
The motivation behind separating default ISM from not is that the default ISM can and will change frequently and users shouldn't need to take action based off that
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.
leaving stale comments for now, will give another pass
HIP-3.md
Outdated
|
||
For a message to be processed, a transaction must be made to the destination chain. Most Hyperlane user’s don’t want to be responsible for the infrastructure or manual effort required in submitting this transaction themselves, and prefer for a general purpose relayer to do this on their behalf. To ensure a relayer is properly compensated for transaction fees and cannot be griefed, messages must pay the relayer. | ||
|
||
Today, an application hoping to calculate the appropriate payment for a message does so entirely off-chain using the Hyperlane SDK. When dispatching the message on the origin chain, some origin chain native tokens are paid to the relayer by calling the payable function `InterchainGasPaymaster.payGasFor`. The relayer observes the `GasPayment` event emitted by that function, and builds a database of payments for each message. The relayer will then continually re-evaluate whether messages are eligible for processing, which occurs if the USD value of the total payment for a message in origin chain native tokens exceeds the USD value of the destination transaction fees. In other words, there’s a social contract between message senders and the relayer that the relayer will submit the process transaction to the destination if it has been properly compensated. |
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.
is native tokens necessarily native fee currency eg ETH? or can relayers accept other fee currencies eg USDC
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.
Yeah a relayer could accept whatever token they want. Though for now I like using native tokens for a few reasons:
- users already have native tokens to pay for gas
- limiting choices can sometimes be nicer for users - reduces the feeling of being overwhelmed by options
- less for us to build - don't need to worry about a lot of exchange rates or another
payGasFor
function - Apps that have users pay for gas don't need to add the
address gasToken
&uint256 gasPayment
params to their own functions, they can just make their functions payable
The off-chain gas payment estimation, while nice for keeping as much data and calculation off chain to limit costs, has a number of downsides: | ||
|
||
- It can be unclear if a message being dispatched will end up being processed by the relayer at its destination. | ||
- Origin/destination chain native token exchange rates or destination chain gas prices can change, causing the USD value of the payment to be insufficient by the time the relayer evaluates the message for processing. A top-up payment would then be required, which is a poor user experience. |
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.
would be interesting to consider stable fee currencies
|
||
An IGP that quotes and requires on-chain payment for a total amount of gas that is to be used by the ISM and recipient `handle` would look like: | ||
|
||
``` |
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.
Mark as solidity
* @param _gas An amount of gas. | ||
* @param _refundAddress The address to refund any overpayment to. | ||
*/ | ||
function payForGas( |
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.
Should there be another implementation of this that omits _gas
and _refundAddress
? For destination chains where on-chain fee quoting isn't happening
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.
To make sure I understand correctly: this would be in a scenario where someone is using our IGP that does perform on-chain fee quoting, but they are calling payForGas
with a domain that doesn't have any oraclized fees / gas prices?
If so, we could make sure that the on-chain-fee-quoting IGP doesn't charge anything for domains it doesn't know any info about? Or possibly a flat default fee that doesn't scale with gas amount.
Or if you mean a situation where someone wants to use their own IGP that doesn't have any on-chain fee quoting support, they could just not use the _gas
and _refundAddress
at all, like we did before https://github.com/hyperlane-xyz/hyperlane-monorepo/blob/6046ce68ef32b986d4ed813ad656d05497911815/solidity/contracts/InterchainGasPaymaster.sol#L50
Maybe I'm misunderstanding what you mean though. But I'm more inclined to have a single function interface if possible to avoid confusion
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.
this would be in a scenario where someone is using our IGP that does perform on-chain fee quoting, but they are calling payForGas with a domain that doesn't have any oraclized fees / gas prices?
Correct. Basically I don't want to add additional requirements for supporting a new chain. Relaying should be able to work in the absence of on-chain oraclization
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.
To underscore, making deploying Hyperlane to new chains as easy as possible is one of the most important things we can do, and it's important that the gas payment protocol aligns with that goal
} | ||
``` | ||
|
||
Hyperlane users are likely to be confident in the amount of gas that their recipient's `handle` function will use on the destination, but they likely don't have the same confidence in the amount of gas they need for their ISM. Users may be using the default ISM which could change entirely, or their ISM may undergo parameter changes that could change the amount of gas required. To provide a better experience, ISM-specific IGPs can be provided that allow users to supply the amount of gas their recipient's `handle` function consumes, and the ISM-specific costs are read from storage and accurately updated by the IGP owner. |
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.
This refers to an "ISM specific IGP" in the singular, but contains a dictionary that is supposed to represent many remote ISMs. Which is it?
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.
Mm good point, this is confusing. I guess a better way of saying this is that an IGP like this would be specific to many remote ISMs that an application is using. So we would provide an IGP that would relate to the default ISMs on many remote chains, because this is what we expect most people to use as they'll likely just be using default ISMs
* @param amount The amount of native tokens paid. | ||
* @param gas The amount of destination gas paid for. | ||
*/ | ||
event GasPayment( |
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.
Flagging that this event does not handle unknown chains gracefully
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.
Would not charging anything for domains it doesn't know any info about or charging some sort of default fee satisfy this for you? as suggested in https://github.com/hyperlane-xyz/hips/pull/3/files#r1073804672
Hyperlane users are likely to be confident in the amount of gas that their recipient's `handle` function will use on the destination, but they likely don't have the same confidence in the amount of gas they need for their ISM. Users may be using the default ISM which could change entirely, or their ISM may undergo parameter changes that could change the amount of gas required. To provide a better experience, ISM-specific IGPs can be provided that allow users to supply the amount of gas their recipient's `handle` function consumes, and the ISM-specific costs are read from storage and accurately updated by the IGP owner. | ||
|
||
``` | ||
contract IsmFeeQuotingIgp { |
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.
Is the idea that there just be one of these that represents default ISMs everywhere? Or do you expect everyone that deploys an ISM to also deploy and configure one of these?
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.
I think we'd want to provide one that represents default ISMs everywhere. And permissionless deployment chains can choose to do the same. And then if someone deploys their own non-default ISM, they can choose to deploy and configure one of these or skip it entirely and go with your suggestion of requiring applications to be aware of the ISM costs themselves. I don't think we need to be prescriptive with it when it comes to non-default ISMs
|
||
#### ISM-specific fee quoting IGPs | ||
|
||
On the origin chain, an IGP cannot know what the length of an arbitrary ISM's metadata calldata is, nor can it know the expected gas used by an arbitrary ISM's `verify` function. Therefore, the IGP must get this information from some source. |
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.
The IGP cannot, but the application sending the message can
|
||
Hyperlane users sending messages that use the default ISM should not need to worry about the destination gas costs incurred by the default ISM. This includes the metadata calldata and the `verify` function. Because the default ISM is out of user control and the parameters of the default ISM may change, a straightforward “happy path” for default ISM users is provided via the default ISM's IGP. | ||
|
||
Any non-default ISMs may also have their own ISM-specific IGPs created. Alternatively, users can always call the fee quoting IGP directly with their own `_ismAndHandleGas` amount. |
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.
Any non-default ISMs may also have their own ISM-specific IGPs created
I still strongly prefer that applications just include Hyperlane overhead in the amount of gas that they pay for
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.
By Hyperlane overhead you mean ISM costs right? Fwiw, they can always opt in to doing that by calling the fee quoting IGP directly.
Imo so much of the value of smart contracts is that you're generally able to just deploy something and it just "works" without worrying about operations / things no longer working. Most apps will just use the default ISM (at least for now), and if that default ISM ever changes, app contracts shouldn't be expected to make any changes on their side. Otherwise every time we want to change the quorum threshold, we risk breaking contracts because they could be specifying too low of a gas amount. If we tell people to use a generous buffer of like ~200k and say that the default ISM costs will never exceed that, it still feels like we're constraining what ISMs can do unnecessarily, while overcharging users.
I guess I see this whole ISM-specific IGP thing as providing the simplest experience for the majority of users while giving us the most flexibility for future changes without disruption. I'm not sure I understand the benefits of applications including the Hyperlane/ISM overhead themselves?
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.
Also fwiw, I feel most strongly about this for the default ISM case where people don't want to have to think about ISMs at all and just want things to work. For more complicated use cases like custom ISMs etc that people explicitly opt into, I feel less strongly. In those situations we can tell them to just call the fee quoting IGP directly and not deal with these gas-overhead-imposing-IGPs
function destinationGas(uint32 _destinationGas) external returns (uint256); | ||
``` | ||
|
||
However, this conflicts with permissionless deployment of Hyperlane onto long-tail chains. A relayer can decide to offer its relaying services to process messages to long tail chains if they so choose, and it's more natural to put the onus on the relayer to specify what those costs are. |
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.
and it's more natural to put the onus on the relayer to specify what those costs are.
Sorry, I'm not sure I follow this? Feels important, can you explaiN?
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.
I'll clarify this in the HIP
I think this is trying to say that because a relayer decides which chains it wants to relay to, it's natural to have the relayer be responsible for maintaining an IGP that has the default ISM gas amounts for the remote chains it supports. And that putting the remote gas amounts inside an ISM doesn't make sense
|
||
The gas used by the message's bytes in the calldata of a process transaction are not explicitly calculated by the InterchainGasPaymaster because the upper limit of the gas cost is low, and performing this calculation on-chain introduces additional storage reads and computational complexity. Additionally, providing the InterchainGasPaymaster with the exact size in bytes of the Hyperlane message isn't straightforward. | ||
|
||
#### Flexibility |
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.
I'd still like to see a token amount only version of the interface, which IMO feels the most flexible
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.
Wondering how you feel about just saying that people can implement and use their own IGP with the same function interface but they could just not care about gas amount / refund address / any on-chain fee quoting like we did before https://github.com/hyperlane-xyz/hyperlane-monorepo/blob/6046ce68ef32b986d4ed813ad656d05497911815/solidity/contracts/InterchainGasPaymaster.sol#L50
Looking for feedback on everything! Some specific things I'd like for you to mull over: