Repository navigation
Conversation
| if (!isReserved) return ok(true); | ||
| const quote = await popRules.priceWithoutCheck.query(label, reservationOwner, { origin }); | ||
| if (!quote.success) { | ||
| if ((quote.value as { type?: unknown } | null)?.type === "ContractRevertedWithPayload") { |
There was a problem hiding this comment.
priceWithoutCheck is a view that returns a Reserved status rather than reverting, so this branch looks unreachable on a valid label and the band closed to sale comment doesnt match PopRules.sol. if you keep it worth reusing isEmptyRevert so it doesnt drift from the helper above. intended?
There was a problem hiding this comment.
It's actually reachable: _priceWithoutCheck calls _requireShortNamesOpen(baseLength) before reading the reservation, which reverts PopError("Short names are not for sale") for a base under 9 characters while shortNamesEnabled is off. It's off on both Polkadot Asset Hub and Paseo today, so a reserved 6 to 8 letter name lands here.
Reusing isEmptyRevert would miss this revert, because it carries a payload. To keep the checks from drifting, both now use a shared isRevert helper.
There was a problem hiding this comment.
Sorry, I forgot to push the fix I was mentioning, it was only local to my laptop. Now pushed in 7102783
TarikGul
left a comment
TarikGul
left a comment
There was a problem hiding this comment.
Checked the abi against PopRules.sol, isBaseNameReserved matches the deployed shape and the reserved status read is right. 92 dotns tests green, changeset patch only. One note on the revert branch inline, not blocking.
📦 Bundle size impactComparing
Thresholds — 🟡 ≥10% or ≥5.0 KB · 🟠 ≥20% or ≥15.0 KB (bundled). Percentage only applies once the baseline is ≥ 10 KB. Informational — this check never blocks merge. |
Problem
isDotNsAvailablereported a name reserved through the gateway as available. Registering it then failed withNameReserved.From dotNS 1.1.0, a live reservation blocks public registration for every account, including the user who holds it (paritytech/dotns#348). Paseo Asset Hub and Polkadot Asset Hub run 1.0.0 until they are upgraded, so the SDK has to handle both.
Fix
isDotNsAvailablenow also readsPopRules.isBaseNameReserved. When the name is reserved, it asks PopRules' price quote for the reservation's holder, and lets that answer decide:isDotNsAvailableReservedtrueReservedfalsefalseRegistryCallerrorprepareDotNsRegistrationalready refuses withNameReservedon the same quote, so registration needs no change.Other changes
isBaseNameReservedadded to the vendored PopRules ABI. It's the same on 1.0.0 and 1.1.0.NameReservedcomment now covers reservations.Testing
pnpm checkpnpm --filter "@parity/product-sdk" test: 208 tests, with new cases for each row of the table and for a failed reservation read.