paritytech / paritytech/dotns

[Bug]: UI - Failed contract reads are silently replaced with fallback values, unlike writes

Open
#227 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

dotns-sdk P2 type: bug
Dominant language
Solidity
Stars
4
Forks
2
Avg merge
1d 18h
Merged PRs (30d)
24

Description

Component

Other

Priority

P2

What happened?

packages/ui handles read and write failures under opposite policies. The write helper fails loudly:

// lib/contractWrite.ts
async function submitWrite(tx, action): Promise<Hash> {
  try { result = await tx; }
  catch (error) { throw new Error(describeContractError(error, action)); }
  if (!result.ok) throw new Error(failure(action, describeDispatchError(result.dispatchError)));
  return result.txHash as Hash;
}

Both failure shapes are covered and the revert bytes are decoded against the contract ABIs before any fallback. The helper is correct; individual call sites can still discard what it throws, so "writes fail loudly" holds for the layer and not for every path through it. Reads do the reverse:

// composables/useContracts.ts:173
export function safeRead<T>(label: string, fallback: T, fn: () => Promise<T>): Promise<T> {
  return withContractRecovery(fn).catch((error) => {
    console.warn(label, error);
    return fallback;
  });
}

An RPC failure, a stale contract address, an ABI mismatch or a revert all become a console.warn and a substituted value. The caller cannot tell a real answer from a swallowed error, and the user sees nothing.

The same commonly happens for other reads and two instances are worth naming.

Availability is decided from the wrong source, which gates the registration funnel. store/useUserStoreManager.ts:141:

const owner = resolvedAddress ?? nameOwner ?? ZERO;   // available = owner === ZERO

This prefers the resolver's addressOf record over the registrar's owner, and getOwnerOfDomain returns null on read failure (store/useResolverStore.ts:114,120), indistinguishable from "no owner". So an owned name reads "available" whenever that read fails, sending the user into a paid register that reverts; a free name with a leftover addr record reads "taken" and blocks a legitimate registration. This store backs the green "Available" in the search box.

The fallback is sometimes structural rather than a value. readAllPages (store/useUserStoreManager.ts:30-36) does if (!page) break;, treating a failed page read as end-of-list and returning a short list as complete. Names or uploads silently disappear, and components/profile/EscrowTab.vue:363 then omits the matching escrow positions, so deposits look missing.

Expected behavior

A failed contract read reaches the user the same way a failed write already does. Where a fallback is a genuine answer, such as "this name has no text record", it is kept. Where absence has no valid representation, such as a price, a tier or a minimum age, the read rejects and the caller surfaces the error.

Reproduction

Point the app at a stale contract address, which is the live situation described in #221, and register a name. The query returns unsuccessful, priceWithoutCheck yields price: 0n, the transaction is submitted with value: 0, and the user sees a contract revert about insufficient value rather than a message about the failed price lookup. The real error appears only in the browser console.

The same is reproducible by blocking the RPC endpoint mid-session: prices display as zero rather than as an error.

Additional context

safeRead's fallback semantics hold where a fallback is a real answer, such as "this name has no text record", and do not hold where no value can represent a failed lookup, such as a price, a tier or a minimum age. Reads in the first group can keep their current behaviour.

That gives a narrow fix: split safeRead into a fallback-preserving variant and a rejecting variant, and move the reads whose absence is unrepresentable onto the latter. The write layer already turns a thrown error into a readable message via describeContractError, so no new error-presentation work is needed.

This composes with two known bugs, which is why it matters for launch. #221 leaves stale contract addresses in place, and #223 means no mainnet configuration exists yet; both produce failing reads, and both currently surface as plausible wrong values rather than as errors.

Scoped to packages/ui deliberately. The CLI has smaller instances of the same class, notably listEscrowPositions swallowing per-name errors and normalizeFlags defaulting to "no revert", and sync-abis failing open is already tracked in #222.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in packages/ui/composables/useContracts.ts at safeRead, then trace its callers in store/useUserStoreManager.ts and store/useResolverStore.ts, including readAllPages and owner resolution. Reproduce the behavior with a stale contract address or a blocked RPC endpoint, and inspect components/profile/EscrowTab.vue for the missing escrow symptom. Done means genuine fallbacks remain available while unrepresentable failed reads surface an error instead of a plausible value.

Written by the indexing model from the issue text.

Assessment

Tech stack
solidity, typescript
Domain
blockchain, frontend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.