extend SMP protocol to support name availability queries - #1863
Conversation
…e and meaningful replies
Co-authored-by: Evgeny <evgeny@poberezkin.com>
Co-authored-by: Evgeny <evgeny@poberezkin.com>
Co-authored-by: Evgeny <evgeny@poberezkin.com>
Co-authored-by: Evgeny <evgeny@poberezkin.com>
Co-authored-by: Evgeny <evgeny@poberezkin.com>
|
tested e2e locally with hardhat mocked cases with dummy values: also, too short names can be resolved, just not registered publicly caveat:
|
| else do | ||
| bs <- brReadSome (responseBody res) (maxResponseBytes + 1) | ||
| pure $ if BL.length bs > fromIntegral maxResponseBytes then Left BodyTooLarge else Right bs | ||
| bs <- brReadSome (responseBody res) (maxResponseBytes + 1) |
| -- | GET <baseUrl><path>, returning the response status and body bytes within the | ||
| -- size cap. Redirects are disabled and Authorization is attached only when | ||
| -- configured. |
There was a problem hiding this comment.
some spurious linebreak changes
| resolveHttp env name = | ||
| (>>= first InvalidJson . J.eitherDecodeStrict . BL.toStrict) | ||
| <$> httpGet env ("/resolve/" <> B.unpack (urlEncode True (encodeUtf8 name))) | ||
| (>>= nameResp) <$> httpGet env ("/resolve/" <> B.unpack (urlEncode True (encodeUtf8 name))) |
There was a problem hiding this comment.
that looks like some drive-by refactoring. What is it doing and why?
| (Nothing, Nothing) -> Left NOT_FOUND | ||
|
|
||
| -- | The resolver's status words. An unknown status is not an answer. | ||
| mapStatus :: Maybe NameRecord -> NameStatusResp -> Either NameErrorType NameRegistration |
There was a problem hiding this comment.
very complex - what is the problem here?
There was a problem hiding this comment.
Looks like translation between what resolver returns and what we want in protocol.
We cannot upgrade all servers at once so what resolver returns should be compatible, so splitting "status" to separate record kind of makes sense from this point of view.
But a better solution would have been exposing a different version endpoint in resolver that responds to old server what it wants and to new server what it wants - the same type we defined elsewhere. This would avoid this mapping entirely.
There was a problem hiding this comment.
I assumed the resolver and SMP relay get updated at the same time as they are shipped together. But yes, we could just introduce a new version in the url path and stay compatible with the old SMP relays. Anyway, I think it is a good idea to let the resolver respond with the data model that can be parsed straight into the SMP type
There was a problem hiding this comment.
ah, now I remember: only one light client and resolver for the cluster, right? Then I suggest we do it properly with a /v2/ path on the resolver
| sendProtocolCommand c nm Nothing NoEntity (Cmd SResolver (RSLV name)) >>= \case | ||
| RNAME nr -> pure nr | ||
| | v >= namesSMPVersion = | ||
| sendProtocolCommand c nm Nothing NoEntity (Cmd SResolver (RSLV (nameQuery v name))) >>= \case |
There was a problem hiding this comment.
this decision is already made during the protocol encoding - why are we doing it here as well?
There was a problem hiding this comment.
It feels like it's a knockdown effect of not putting the right type into the protocol. If we say that whether we use hash or name is version dependent, then RSLV should just accept SimplexDomain and compute its hash during encoding.
| RFWD :: EncFwdTransmission -> Command ProxyService -- use CorrId as CbNonce, proxy to relay | ||
| -- Resolve SimpleX name. | ||
| RSLV :: SimplexDomain -> Command Resolver | ||
| RSLV :: NameQuery -> Command Resolver |
There was a problem hiding this comment.
see comment above - I think it should remain SimplexDomain?
| -- hash, knows neither. The formula is in protocol/simplex-messaging.md. | ||
| data NamePricing = NamePricing | ||
| { -- | US cents per year, for the lengths the registry prices specially | ||
| rentPrices :: Map Int USDCents, |
There was a problem hiding this comment.
rentPrices -> registrationPrices
|
|
||
| -- | What RSLV asks about. Unlike SimplexDomain, which is always text, this may | ||
| -- name a label by its hash. | ||
| data NameQuery = NameQuery |
There was a problem hiding this comment.
I think the main source of the messiness and spread of version is this type - instead of being a plain sum, where a name is either domain or hash of domain it tries to make some merged shape that only complicates things. E.g., we know that we never need to look up subdomains as hashes, but this type allows it.
A much simpler type would have been this:
data NameQuery = NQDomain SimplexDomain | NQHash SimplexTLD LabelHash
Then conversion from name to query is always version independent, it's just NQDomain.
Encoding then remains version dependent, and it can send hash only if 1) version supports it 2) it's second level name 3) it's not web name. This would avoid multiple decision points that are currently present about how to encode the name, hashing would become part of encoding.
There was a problem hiding this comment.
this also simplifies encodings, as it doesn't have to encode components separately - it only needs one-letter tag + either domain (that we already know how to encode) or TLD + hash.
Let users query name availability by labelhash (without revealing plaintext name unless it resolves)
They should get a precise answer with details in all possible cases (expired, grace, auction, reserved)
Assumes these get merged and deployed for .simplex, but PR works against .testing names as well.
assumes merged implement configurable prices for oracle simplex-network/ens-contracts#28
assumes merged implement reservation reason enum simplex-network/ens-contracts#30
bump SMP protocol version