Persist txReceipt on txMeta for confirmed txs#1592
Conversation
OGPoyraz
force-pushed
the
1079-add-txreceipt-to-transaction-state-in-core-transaction-controller
branch
from
b2b249b to
696c007
Compare
| */ | ||
| export interface TransactionReceipt { | ||
| /** | ||
| * The block hash of the block that this transaction was included in |
There was a problem hiding this comment.
As mentioned on one of the other PRs, can we go with . at the end of these descriptions so it's easier to use punctuation like commas for example?
| * @type TransactionReceipt | ||
| * | ||
| * Following properties are not included in the type definition but are returned by the API: | ||
| * @property byzantium - True if the block is in a post-Byzantium Hard Fork block. |
There was a problem hiding this comment.
If these are consistently returned and we're documenting them anyway, is it cleaner to just include them as standard properties with inline descriptions?
There was a problem hiding this comment.
I don't think all of them consistently returned from the API, moreover I see some differences in the TxReceipt type between versions of provider. Since provider (actually the object we use for the queries) is passed over while instance creation, this type is coupled with the version of that. In this situation, we cant keep track of these API additions in long term tbh.
I've added this list but now I feel like remove unused properties completely.
There was a problem hiding this comment.
Definitely agree, I think it adds too much ambiguity if we have two sets of values and descriptions.
Shall we just remove these JSDoc properties and just stick with the inline properties and descriptions we have?
| } | ||
|
|
||
| /** | ||
| * @type TransactionReceipt |
There was a problem hiding this comment.
I don't think we need this type parameter, but just a little description for the type like Standard data concerning a transaction processed by the blockchain for example.
| } | ||
|
|
||
| /** | ||
| * @type Log |
There was a problem hiding this comment.
Same here, should we just include the description?
OGPoyraz
force-pushed
the
1079-add-txreceipt-to-transaction-state-in-core-transaction-controller
branch
from
adc7a9e to
7c5b84e
Compare
…ore-transaction-controller
OGPoyraz
deleted the
1079-add-txreceipt-to-transaction-state-in-core-transaction-controller
branch
3 participants
Explanation
This PR aims to persist
txReceiptontxMetaafter checking blockchain for confirmed transaction.Changelog
@metamask/transaction-controllertxReceiptontxMetafor confirmed txsChecklist