Chore: finish networks refactoring #14398
Conversation
|
|
||
| // increment the appropriate counter based on the network symbol | ||
| const incrementAccumulator = () => { | ||
| if (symbol === 'ada' || symbol === 'tada') acc.numberOfCardano += 1; |
There was a problem hiding this comment.
Note: at first I thought just including 'tada' here would solve the 🎉 🐛,
but I also had to do the "1 or 2 thing" with numberOfCardanoCoins instead of always 1.
Otherwise, with ada & tada at the same time, discovery was wrongly indicated as finished, with debug showing 16 / 6 as loaded / total 😱
I would have to activate some more coins to start the discovery of ada or tada..
| @@ -560,8 +560,7 @@ const intlMock = { | |||
| formatMessage: (s: any) => s.defaultMessage, | |||
| }; | |||
|
|
|||
| const mockedBlockchainNetworks = networksCompatibility.reduce((result, network) => { | |||
| if (network.accountType) return result; | |||
There was a problem hiding this comment.
this line means that when using networksCompatibility, only 'normal' accounts were processed, which makes the refactoring really easy
| ...networkOverride, | ||
| })), | ||
| ], | ||
| ); |
| @@ -33,7 +33,7 @@ export const disableAccountsThunk = createThunk( | |||
| const accounts = selectAccounts(getState()); | |||
| const enabledNetworks = selectEnabledNetworks(getState()); | |||
| // find disabled networks | |||
| const disabledNetworks = networksCompatibility | |||
| const disabledNetworks = networksCollection | |||
| .filter(n => !enabledNetworks.includes(n.symbol) || n.isHidden) | |||
| .map(n => n.symbol); | |||
There was a problem hiding this comment.
ℹ️ this is different! With networksCompatibility, the array of symbols is not unique, so e.g. 'btc' is duplicated foreach account. Now with networksCollection it is unique.
BUt in this case it's ok, see L41, disabledNetworks is only used to test .includes
| @@ -61,7 +61,7 @@ export const preloadFeeInfoThunk = createThunk( | |||
| `${BLOCKCHAIN_MODULE_PREFIX}/preloadFeeInfoThunk`, | |||
| async (_, { dispatch }) => { | |||
| // Fetch default fee levels | |||
| const networks = networksCompatibility.filter(n => !n.isHidden && !n.accountType); | |||
There was a problem hiding this comment.
filters only 'normal' accounts, so it's the same ✔️
|
|
||
| if (!account || !networkFeeInfo || !device || !network) return null; | ||
| const network = networks[account.symbol]; |
There was a problem hiding this comment.
when !!account, network is now guaranteed ✔️
| } else { | ||
| acc.numberOfNonCardano += 1; | ||
| } | ||
| const device = selectDevice(getState()); |
There was a problem hiding this comment.
what's going on here? See L741, L748 where it's first called.
Previously, networksSymbols from filterUnavailableNetworksCompatible was non-unique, like in comment above, except here it actually mattered, and imho the previous implementation was kinda occult.
In calculateProgress, we need the number of loaded accounts, so the number of duplicated discovery.networks symbols was counted.
Now, discovery.networks is unique, and it is responsibility of calculateProgress to iterate through accounts.
💡 Note how filterUnavailableNetworksCompatible is split between filterUnavailableNetworks and filterUnavailableAccountTypes.
Finish refactoring of old
networksCompatibletonetworks🎉🚧 Depends on #14281, #14374, so if you want to CR this in advance, then only last 3 commits.
Description
selectedAccountstatetada, discovery would seemingly never finishRelated Issue
Resolve #13839 , finally☺️ 🎉
Screenshots:
I tested in app that the discovery works as before: all accounts are discovered & unfinished/in progress is indicated correctly. But to be really sure, I console logged
loadedandtotalatcalculateProgress#L147btc, ltc, eth, sol, test, regteston T2T1 w/ academic seed, and this is how the discovery progressed on this branch vs. develop: discovery test.txt65 / 65both this branch & develop92 / 92both this branch & develop16 / 36TADA 🎉 bug: wrongly indicated as unfinished even when actually finished:
