fix: historical queries for account balances

[TarikGul]

closes: #634

This fixes historical queries for /accounts/{accountId}/balance-info.

[TarikGul]

I need to confirm that this works for all runtimes before block ~1500000 (Kusama), currently they all work except when querying block 1400000's runtime.

[TarikGul]

All historical runtimes are now covered.

The initial issue was present in two cases: For example, when querying Kusama, the following 2 groups of runtimes have different storage formats.

v1054 --> v1050
v1049 --> ... (earliest runtime)

---

Presently we use 2 formats to get account storage information:

a) api.query.system.accounts.at(hash, address)
b) api.query.balances.account.at(hash, address)

But for runtime v1054 --> v1050 the api requires a historicalApi via await api.at(${BlockHash})

historicalApi.query.system.account(address)

And for runtime v1049 --> ...(earliest runtime) also requires a historicalApi

historicalApi.query.balances.freeBalance(address),
historicalApi.query.balances.locks(address),
historicalApi.query.balances.reservedBalance(address),
historicalApi.query.system.accountNonce(address),

[emostov]

I wonder if this is necessary, could we just do null if one of these fields doesn't exist?

[TarikGul]

When you say 'do null' do you mean a null response for the key value pair? Or do you mean just check one value and then throw the error.

Here I am just mimicking the error response logic that is in the existing service to keep it consistent with what response/errors it reports. So if we change this to something different it might be worth updating the rest as well.

[TarikGul]

Ahh, I think I see what you are saying, It only makes sense to really check for one value. Will update

[emostov]

LGTM % a few questions where we might be able to improve performance. Also, if these derives https://github.com/polkadot-js/api/blob/master/packages/api-derive/src/balances/account.ts start supporting .at I wonder if it would be more robust to start using them in parts of this service in the future

[TarikGul]

Also, if these derives https://github.com/polkadot-js/api/blob/master/packages/api-derive/src/balances/account.ts start supporting .at I wonder if it would be more robust to start using them in parts of this service in the future

Yea I totally agree with this.

[maciejhirsz]

Looks good to me.

There is a bunch of places where you mock an async function by chaining a Promise.resolve with no value, and I'm not sure it's necessary when a regular async function could be used?

[maciejhirsz]

Wouldn't that be easier?

Suggested change

	freeBalance: () =>
	Promise.resolve().then(() => {
	return polkadotRegistry.createType('Balance', 123456789);
	}),
	freeBalance: async () => polkadotRegistry.createType('Balance', 123456789),

[TarikGul]

Nice catch, yea I definitely agree, will fix it up now.

[TarikGul]

fixed