fix speed up/cancel transaction #1413
Conversation
|
Here's the packed extension for this build: |
greg-schrammel
force-pushed
the
gregs/bx-1327-speed-up-tx-rejected-by-rpc-for-0-gas-extension-crash-on
branch
from
60e651c
to
220acdd
Compare
greg-schrammel
changed the title
greg-schrammel
force-pushed
the
gregs/bx-1327-speed-up-tx-rejected-by-rpc-for-0-gas-extension-crash-on
branch
from
220acdd
to
8d35099
Compare
|
Here's the packed extension for this build: |
greg-schrammel
changed the title
|
Here's the packed extension for this build: |
greg-schrammel
changed the title
|
Here's the packed extension for this build: |
greg-schrammel
force-pushed
the
gregs/bx-1327-speed-up-tx-rejected-by-rpc-for-0-gas-extension-crash-on
branch
from
0a91cf7
to
4882311
Compare
|
Here's the packed extension for this build: |
…p-tx-rejected-by-rpc-for-0-gas-extension-crash-on
src/entries/popup/handlers/wallet.ts
Outdated
| } | ||
| }; | ||
|
|
||
| function deserializeBigNumbers<T>(obj: T) { |
There was a problem hiding this comment.
not 100% sure about this solution, open to suggestions
but walletAction bridges to the background worker and back, in this back and forth it serializes BigNumbers as { type: 'BigNumber', _hex: "0x..." }
so sometimes we got an [object Object] instead of the bignumber in some places
There was a problem hiding this comment.
ah yes i've seen this, maybe let's just use hex strings? only sending hex from popup to background and only sending hex from background to popup so we don't have to do this
There was a problem hiding this comment.
I started to do it and got afraid of changing the type and breaking in unexpected places, because it's being casted as TransactionResponse and that is why we didn't caught it for so long to begin with
to do it without changing the type, I'd have to then deserialize by hand like
return {
...response,
maxBaseFee: BigNumber.from(response.maxBaseFee)
...
}which is more error prone and effort
another idea is to do this generic BigNumber deserialization inside walletAction so we don't have to ever think about it again in other places
There was a problem hiding this comment.
okok sounds good, can we move this to a util file somewhere else?
| }} | ||
| title="Failed to speed up transaction" | ||
| description={[ | ||
| 'A speed up can fail when the state of the network changed since you first submitted the transaction', |
There was a problem hiding this comment.
@DanielSinclair let's define a copy for this and I add to the translations
There was a problem hiding this comment.
@greg-schrammel Some copy:
"Speed up failed"
"Attempts to speed up a transaction can fail during network congestion, gas limit increases, or when the original transaction is confirmed first"
greg-schrammel
requested review from
DanielSinclair,
estebanmino,
derHowie and
BrodyHughes
greg-schrammel
changed the title
|
Here's the packed extension for this build: |
|
bumping this small comment #1413 (comment) |
…for-0-gas-extension-crash-on
|
Here's the packed extension for this build: |
|
Here's the packed extension for this build: |
greg-schrammel
deleted the
gregs/bx-1327-speed-up-tx-rejected-by-rpc-for-0-gas-extension-crash-on
branch
|
Here's the packed extension for this build: |
Co-authored-by: Esteban Miño <[email protected]> Co-authored-by: Daniel Sinclair <[email protected]> Co-authored-by: brdy <[email protected]> Co-authored-by: brunobar79 <[email protected]> Co-authored-by: MK <[email protected]> Co-authored-by: Magomed Khamidov <[email protected]>
Fixes BX-####
Figma link (if any):
What changed (plus any additional context for devs)
Screen recordings / screenshots
What to test
try to speed up and cancel a transaction