-
Notifications
You must be signed in to change notification settings - Fork 78
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
fix: wallet connect - sake.lido.fi dApp - staking transaction gets stuck #16348
Conversation
maxFeePerGas = hexToGwei(tx.maxFeePerGas) | ||
maxPriorityFeePerGas = hexToGwei(tx.maxPriorityFeePerGas) | ||
|
||
// TODO: check why we need to set gasPrice here and why if it's not checked we cannot send the tx and fees are unknown???? | ||
gasPrice = hexToGwei(tx.maxFeePerGas) |
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.
Not sure why gasPrice
is used in this context (refer to TODO in the line above)?
@@ -36,15 +36,15 @@ template getProp(obj: JsonNode, prop: string, value: var typedesc[uint64]): bool | |||
|
|||
template getProp(obj: JsonNode, prop: string, value: var typedesc[string]): bool = | |||
var success = false | |||
if (obj.kind == JObject and obj.contains(prop)): | |||
if (obj.kind == JObject and obj.contains(prop) and obj[prop].kind == JString): |
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.
All functions in this file should have this check and obj[prop].kind == ....
but when I did that I couldn't start the app (got crash) which undoubtedly means that in some places we have incorrect type usage or receiving incorrect json data. That should be fixed in another PR.
Jenkins Builds
|
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 staking tx works on my end
@@ -484,13 +484,11 @@ SQUtils.QObject { | |||
|
|||
// Beware, the tx values are standard blockchain hex big number values; the fees values are nim's float64 values, hence the complex conversions | |||
if (!!tx.maxFeePerGas && !!tx.maxPriorityFeePerGas) { |
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 we are ine post eip1559 case, both are present, we do not need to set gas price i believe
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.
exactly, that was my question about, here.
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.
LGTM!
Placed tx:
Fixes #16096