hasTokenBalance does not correctly detect token balances

[turbocrime]

hasTokenBalance contains incorrect logic

web/packages/storage/src/indexed-db/index.ts

Lines 872 to 887 in d18f2d0

	async hasTokenBalance(addressIndex: AddressIndex, assetId: AssetId): Promise<boolean> {
	const spendableNotes = await this.db.getAllFromIndex(
	'SPENDABLE_NOTES',
	'assetId',
	uint8ArrayToBase64(assetId.inner),
	);
	return spendableNotes.some(note => {
	const spendableNote = SpendableNoteRecord.fromJson(note);
	return (
	spendableNote.heightSpent === 0n &&
	!isZero(getAmountFromRecord(spendableNote)) &&
	spendableNote.addressIndex?.equals(addressIndex)
	);
	});
	}

problems

does not query notes table by valid index

web/packages/storage/src/indexed-db/index.ts

Line 876 in d18f2d0

uint8ArrayToBase64(assetId.inner),

an incoming asset id may not possess a binary 'inner' field, but may contain a altBech32m or altBaseDenom field. idb is indexed by the binary inner, but hasTokenBalance does not ensure the presence of this field before using an asset id to query.

performs invalid comparison of address indicies

web/packages/storage/src/indexed-db/index.ts

Line 884 in d18f2d0

spendableNote.addressIndex?.equals(addressIndex)

an address index contains an integer field that may be undefined, and a randomizer that may not be relevant for certain comparison purposes. hasTokenBalance fails to compare zero/undefined equally, and will reject mismatched randomizers

effects

this results in hasTokenBalance failing to correctly determine the presence of a token in an account when it queries idb, depending on the input.

on conditions that are present in the view service now: for some transactions, the view service planner will incorrectly determine that the planner-specified (or native) fee is not present. so,

transactions may break

the planner will enter alt fee selection logic which uses this same broken method to identify presence of tokens. the planner may fail to discover an alternate fee token due to the same issue, and throw the rpc call.

the planner will not always respect the transaction plan

the planner will enter alt fee selection logic, and may be able to discover an alternate fee that disrespects the user's transaction plan.

wasm building may fail

or, the planner may provide uncomparable asset ids or asset ids missing a binary inner to the wasm planner, which seems to have some similar or related problems querying idb.

suggested correction

require input parameters of hasTokenBalance to contain a more restricted message type

addressIndex parameter should be { account: number, randomizer: never }

caller will be responsible for constructing correct input

i believe randomizer is not relevant for this specific comparison. please correct me if this is wrong.

assetId parameter should be passet1${string} or { inner: Uint8Array } and method should assert length

caller will be responsible for constructing correct input

possible related issues

wasm planner cannot always identify correct gas fee

• wasm planner cannot identify gas prices if gas prices are not present in idb. this is incorrect, wasm planner should default to zero gas prices.
• wasm planner may be incorrectly expecting a binary-inner field of assetid input to be present.
• wasm planner may be failing to confirm it is querying idb with valid assetId.inner

[TalDerei]

this is suitable follow-up work, but doesn't actual lead to failures at genesis from what I can tell.

[turbocrime]

update: address-index comparison is fixed by #1605 but remaining issues exist

action item: enforce correct assetid parameter format