diff --git a/protocol/simplex-messaging.md b/protocol/simplex-messaging.md index ac70308e4..3350b391e 100644 --- a/protocol/simplex-messaging.md +++ b/protocol/simplex-messaging.md @@ -1648,6 +1648,7 @@ rownd = %s"ROWND" SP ownedNames | Field | JSON type | Constraints | |---|---|---| +| `lastBlockTs` | number | the oldest block any read behind this answer saw | | `names` | array | the names the address holds, each the `nameResponse` resolving it answers with | | `inUse` | boolean | whether the account has been used at all | | `nextOffset` | number | cursor to resume from, absent when the listing is complete | @@ -1658,6 +1659,12 @@ maintained on expiry, so the registrar still enumerates a name past its grace; it answers as `available` and names nothing the account holds, so it is left out of `names` while still counting towards `inUse`. +As with `RNAME`, a router reads through a node of its own, which can lag. Every +name carries the block it was read at, and `lastBlockTs` is the oldest of those +and of the enumeration's own block, so a client can tell a badly lagging router +even from an answer that carries no names — which is the answer a recovery scan +acts on. + `inUse` is what the registry could see on its own chain — the account's nonce, its balance, and every name it holds, which is not only the names listed here: a later page of a held account still reports `inUse` true with an empty `names`. Holding a name is only one way for an account diff --git a/scripts/resolver/README.md b/scripts/resolver/README.md index 209beecb5..dafe139c5 100644 --- a/scripts/resolver/README.md +++ b/scripts/resolver/README.md @@ -176,7 +176,8 @@ command** section of [`protocol/simplex-messaging.md`](../../protocol/simplex-messaging.md). Answering each name in full is what lets a caller list and act on them without a -second request for each. Enumeration comes off the registrar's ERC-721 index, so +second request for each. `lastBlockTs` is the oldest block any of the reads saw, +so an answer with no names still says how far behind the node was. Enumeration comes off the registrar's ERC-721 index, so a name acquired by transfer counts; one past its grace is still enumerated but answers as available, so it is left out while still counting towards `inUse`. diff --git a/scripts/resolver/service/snrc-resolve.py b/scripts/resolver/service/snrc-resolve.py index f4e9348c3..1c3880d09 100755 --- a/scripts/resolver/service/snrc-resolve.py +++ b/scripts/resolver/service/snrc-resolve.py @@ -898,6 +898,8 @@ def owned_by(address: str, offset: int = 0): Enumeration is read off the ERC-721 registrar, so a name acquired by transfer counts. Each name is answered with the NameResponse /v2/resolve gives for it, so a caller can list and act on them without asking again. + `lastBlockTs` is the oldest block any of those reads saw, so a caller can + tell a resolver lagging critically behind even when it holds no names. `inUse` is what a recovery scan asks, derived from this chain's nonce and balance and every name it holds, not only the page returned: an account holding only other tokens is not seen, and neither is one used on another @@ -926,6 +928,9 @@ def owned_by(address: str, offset: int = 0): "configuredTlds": [], } + # the block the enumeration below is read at; each name resolved after it + # reports its own, and the oldest of them all is what the answer carries + last_block_ts = chain_now() names, truncated, total_held = [], False, 0 for tld, registrar in configured.items(): held = decode_uint(eth_call(registrar, selector("balanceOf(address)") + encode_address(address))) @@ -945,12 +950,15 @@ def owned_by(address: str, offset: int = 0): # available, which says nothing about the token it is enumerated on if status == 200 and body["registration"]["type"] == "registered": names.append(body) + if body["lastBlockTs"] is not None: + last_block_ts = min(last_block_ts, body["lastBlockTs"]) nonce = decode_uint(rpc("eth_getTransactionCount", [address, "latest"])) balance = decode_uint(rpc("eth_getBalance", [address, "latest"])) names.sort(key=lambda n: n["registration"]["nameRecord"]["name"]) return 200, { "address": address, + "lastBlockTs": last_block_ts, "names": names, "nonce": nonce, "balance": str(balance), diff --git a/scripts/resolver/service/test_snrc_resolve.py b/scripts/resolver/service/test_snrc_resolve.py index 88c4d6c3a..49d8c3415 100644 --- a/scripts/resolver/service/test_snrc_resolve.py +++ b/scripts/resolver/service/test_snrc_resolve.py @@ -1047,22 +1047,25 @@ class OwnedByTests(unittest.TestCase): # keccak-256("alice") ALICE = 0x9C0257114EB9399A2985F8E75DAD7600C5D89FE3824FFA99EC1C3EB8BF3B0501 + BLOCK_TS = 1813000000 + def setUp(self): - self._saved = (snrc.REGISTRARS, snrc.eth_call, snrc.rpc, snrc.registration) + self._saved = (snrc.REGISTRARS, snrc.eth_call, snrc.rpc, snrc.registration, snrc.chain_now) snrc.REGISTRARS = {"testing": self.REGISTRAR, "simplex": ""} snrc.rpc = lambda method, params: "0x0" + snrc.chain_now = lambda: self.BLOCK_TS # resolving one name is RegistrationV2Tests' subject; what owned-by adds # is which names to resolve, so the registration itself is stubbed snrc.registration = lambda name: (200, self.response(name, "registered")) def tearDown(self): - snrc.REGISTRARS, snrc.eth_call, snrc.rpc, snrc.registration = self._saved + snrc.REGISTRARS, snrc.eth_call, snrc.rpc, snrc.registration, snrc.chain_now = self._saved - def response(self, name, type_): + def response(self, name, type_, block_ts=None): reg = {"type": type_} if type_ == "registered": reg["nameRecord"] = {"name": name} - return {"lastBlockTs": 1813000000, "registration": reg} + return {"lastBlockTs": self.BLOCK_TS if block_ts is None else block_ts, "registration": reg} def _chain(self, held, label=b"alice"): def call(to, data): @@ -1153,6 +1156,22 @@ class OwnedByTests(unittest.TestCase): self.assertEqual(status, 200) self.assertEqual(len(body["names"]), 1) + def test_an_account_with_no_names_still_reports_the_block_it_was_read_at(self): + """inUse false is the answer a scan acts on, so it has to carry how + stale the registry it came from is.""" + snrc.eth_call = self._chain(0) + _, body = snrc.owned_by(self.ADDR) + self.assertEqual(body["names"], []) + self.assertEqual(body["lastBlockTs"], self.BLOCK_TS) + + def test_the_oldest_block_of_every_read_is_the_one_reported(self): + """A name resolved against a node further behind is what the caller + has to judge the whole answer by.""" + snrc.eth_call = self._chain(1) + snrc.registration = lambda name: (200, self.response(name, "registered", self.BLOCK_TS - 600)) + _, body = snrc.owned_by(self.ADDR) + self.assertEqual(body["lastBlockTs"], self.BLOCK_TS - 600) + def test_a_negative_offset_is_refused(self): snrc.eth_call = self._chain(1) status, body = snrc.owned_by(self.ADDR, -1) diff --git a/src/Simplex/Messaging/Names/Record.hs b/src/Simplex/Messaging/Names/Record.hs index 03a23acf3..f62ea8ab4 100644 --- a/src/Simplex/Messaging/Names/Record.hs +++ b/src/Simplex/Messaging/Names/Record.hs @@ -93,7 +93,9 @@ data NameRegistration -- | What the registry holds for an address: the names it owns, each as the 'NameResponse' resolving it gives, and whether the account is in use data OwnedNames = OwnedNames - { ownNames :: [NameResponse], + { -- | the oldest block any of the reads behind this answer saw, so a lagging registry shows even where no name does + ownLastBlockTs :: Maybe SystemSeconds, + ownNames :: [NameResponse], ownInUse :: Bool, -- | the cursor to resume from, absent when the listing is complete ownNextOffset :: Maybe Int diff --git a/tests/RSLVTests.hs b/tests/RSLVTests.hs index 6471fa8e3..b28d38c15 100644 --- a/tests/RSLVTests.hs +++ b/tests/RSLVTests.hs @@ -160,7 +160,7 @@ testRownUnsupported = -- | Each owned name is the same NameResponse /v2/resolve answers with. ownedBody :: LB.ByteString -ownedBody = "{\"names\":[" <> registeredBody testNameRecord <> "],\"inUse\":true,\"nextOffset\":null}" +ownedBody = "{\"lastBlockTs\":1813000000,\"names\":[" <> registeredBody testNameRecord <> "],\"inUse\":true,\"nextOffset\":null}" ownedName :: NameResponse -> Text ownedName = \case