-
Notifications
You must be signed in to change notification settings - Fork 76
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
14166 add l1 fees #14621
14166 add l1 fees #14621
Conversation
Jenkins BuildsClick to see older builds (12)
|
@@ -1083,13 +1080,15 @@ QtObject: | |||
let (ethCurrency, fiatCurrency) = self.create0CurrencyAmounts() | |||
return ComputeFeeArgs(ethCurrency: ethCurrency, fiatCurrency: fiatCurrency, errorCode: errorCode) | |||
|
|||
# Returns eth value with l1 fee included | |||
proc computeEthValue(self:Service, gasUnits: int, suggestedFees: SuggestedFeesDto): float = | |||
try: | |||
let maxFees = suggestedFees.maxFeePerGasM | |||
let gasPrice = if suggestedFees.eip1559Enabled: maxFees else: suggestedFees.gasPrice |
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 line looks misleading. I mean, how maxFeePerGasM
can be used as gasPrice
?
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.
For EIP-1559 maxFeePerGasM is the absolute maximum you are willing to pay per unit of gas to get your transaction included in a block. You are sure that you will not pay more than that.
So, I understand that you will pay no more than: gas* maxFeePerGasM
For legacy tx you just pay gas*gasPrice.
This is how I understand this.
error "Can't find suggested fees for chainId", chainId=chainId | ||
return | ||
return buildTransaction(parseAddress(addressFrom), 0.u256, $gasUnits, | ||
if suggestedFees.eip1559Enabled: "" else: $suggestedFees.gasPrice, suggestedFees.eip1559Enabled, |
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.
@endulab gas price should not depend on eip1559 and should not be empty.
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 gas price is needed for eip1559 tx?
@endulab you're absolutely right, I overlooked something. |
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.
Looks good. Was there an issue for this or it's just a fix/update to the last one?
Also, is there a status-go PR too?
@jrainville what do you mean? |
4313176
to
b895efa
Compare
…tions There is only one status-go call for all fees: suggested fees, gas and l1 fee. Issue #14166
93e919e
to
53cf65e
Compare
53cf65e
to
8e87fc9
Compare
What does the PR do
All fees (suggested fees, l1 fee, gas units) are computed in 1 status-go call.
Suggested fees contains correct l1 fee.
We display the cost with included l1 fee.
Related status-go pr