algod: Refactor AccountData conversion

[Eric-Warehime]

Summary

Because of the way AccountData conversion was initially implemented--ledgercore.AccountData=>basics.AccountData=>model.Account--we were losing data on App and Asset created/opted into.

The combination of AssignAccountData which throws away AppParams, AppLocalState, AssetHolding, and AssetParams, and AccountDataToAccount which uses those parameters to calculate asset/app numbers, caused that data to be lost.

I've refactored into a separate conversion function directly from ledgercore.AccountData to model.Account. I've also updated the test to cover exactly the data that indexer requires to be correct.

[codecov]

Codecov Report

Merging #5098 (17652f1) into master (5511cf3) will increase coverage by 0.01%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master    #5098      +/-   ##
==========================================
+ Coverage   53.40%   53.42%   +0.01%     
==========================================
  Files         431      431              
  Lines       54381    54420      +39     
==========================================
+ Hits        29042    29072      +30     
- Misses      23079    23085       +6     
- Partials     2260     2263       +3     

Impacted Files	Coverage Δ
daemon/algod/api/server/v2/test/helpers.go	75.33% <ø> (ø)
daemon/algod/api/server/v2/account.go	76.14% <65.85%> (-1.38%)	⬇️
daemon/algod/api/server/v2/delta.go	71.42% <100.00%> (-0.58%)	⬇️
crypto/merkletrie/trie.go	66.42% <0.00%> (-2.19%)	⬇️
crypto/merkletrie/node.go	91.62% <0.00%> (-1.87%)	⬇️
network/wsNetwork.go	64.98% <0.00%> (-0.19%)	⬇️
catchup/service.go	69.64% <0.00%> (+0.47%)	⬆️
data/transactions/verify/txn.go	70.94% <0.00%> (+0.48%)	⬆️
cmd/tealdbg/debugger.go	72.44% <0.00%> (+0.78%)	⬆️
agreement/proposalManager.go	98.03% <0.00%> (+1.96%)	⬆️
... and 2 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[winder]

I updated the indexer validatedblock objects with these changes: algorand/indexer#1455

It looks good, but you can see that the slices are no longer sorted, which is one difference between accountDataToAccount and AccountDataToAccount. This isn't necessarily a problem, just something I noticed.

Do you think it makes sense to keep using AccountDataToAccount but replace ledgercore.AssignAccountData with an AccountDataToAccountData function? I'm fine with either approach.

[winder]

Might be worth putting a comment cross linking the model.Account init in AccountDataToAccount so that the next person in this code knows to update both.

[Eric-Warehime]

Ended up moving it to account.go since it sort of just belongs there.

[shiqizng]

Thanks!

[Eric-Warehime]

Do you think it makes sense to keep using AccountDataToAccount but replace ledgercore.AssignAccountData with an AccountDataToAccountData function? I'm fine with either approach.

I'd like to just get rid of the intermediate type and directly convert from one to the other.