metamask-extension
metamask-extension copied to clipboard
Additional incoming transactions support
Currently only the built-in Infura networks - and not EVM compatible networks where the official Etherscan API is available - support displaying incoming transactions.
This PR proposes introducing additional support for them:
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.
I have read the CLA Document and I hereby sign the CLA
@PeterYinusa did you have a chance to look at this?
@PeterYinusa is there any additional work to do on my side to get it reviewed? Thank you.
@tgmichel sorry we didn't review this yet, looks like a pretty amazing contribution that I'm sure our users would love. Could you do me a favor and rebase this? One area that has changed dramatically is the network.js constants file which is now typescript so you'll have to introduce some types to your changes. Let me know if you need assistance and tag me for review when you're ready :)
@brad-decker Thank you, very excited to finally get this in :) and sorry it took a while, just rebased.
@brad-decker I can take a look
From QA looks good! @brad-decker @tgmichel ! I've only noticed one small behaviour on Polygon Testnet. Bascially, until I don't refresh the browser page I don't see any tx coming through. Not sure why I can only observe this on Polygon Testnet. Any ideas?
Just curious. I would still approve the PR from QA side, since after refreshing, the tx appear.
https://user-images.githubusercontent.com/54408225/201342105-29193b1b-1517-4abb-a0d4-858e9e1db83b.mp4
Thats peculiar. @seaona lets create a ticket to investigate that and i'll get another set of eyes on this.
@brad-decker the issue @seaona mentioned doesn't seem to originate in this PR, is there something I can do to help you unlock it?
The logic for triggering _onLatestBlock
and updating incomingTxLastFetchedBlockByChainId
LGTM. I didn't debug it but it might be worth looking at getSelectedAddress
here:
this._onLatestBlock = async (newBlockNumberHex) => {
const selectedAddress = this.preferencesController.getSelectedAddress();
const newBlockNumberDec = parseInt(newBlockNumberHex, 16);
await this._update(selectedAddress, newBlockNumberDec);
};
If the selectedAddress
is not properly retrieved from preferencesController
, the _update
function will early return:
if (
!Object.hasOwnProperty.call(ETHERSCAN_SUPPORTED_NETWORKS, chainId) ||
!address
) {
return;
}
I'm thinking maybe after refreshing, the selectedAddress
is correctly picked up and works as expected. Anyway this is just a wild guess because if that would happen shouldn't be something network-specific (and apparently is only observed in Polygon Testnet)
Hello @tgmichel Thanks for your PR ! Do you think it's possible to add also incoming transactions for localhost with ganache ? Then it will help when we test locally.
add also incoming transactions for localhost with ganache
The current controller for incoming transactions leverages Etherscan Api, which is not available for Ganache afaik.
@tgmichel @brad-decker @seaona @PeterYinusa
Can i add a bounty to get this expedited? When will incoming transactions be merged? The community has been waiting far too long for such an essential feature to be added
@brad-decker sorry to come back at you again regarding this PR, please let me know when is ready to be merged so I can rebase once. Thank you!
@tgmichel ready for merge, @pedronfigueiredo and I will get this merged once the rebase occurs
@brad-decker done
@pedronfigueiredo when you come online please review and merge :D