store: read only a dataless revert as no owner

apps/simplex-store-service/src/StoreService/Names/Chain.hs

@@ -30,7 +30,7 @@ where
 
 import Control.Concurrent.STM (TVar)
 import Control.Exception (evaluate, try)
-import Control.Monad ((>=>))
+import Control.Monad ((<=<), (>=>))
 import Crypto.Random (ChaChaDRG)
 import Data.Aeson ((.:), (.:?), (.=))
 import qualified Data.Aeson as J
@@ -55,7 +55,7 @@ import Simplex.Messaging.Util (tshow)
 import StoreService.Config (ChainConfig (..))
 import StoreService.Log (logWarn)
 import StoreService.Names.Abi (hex)
-import StoreService.Names.Revert (RevertName, decodeRevert)
+import StoreService.Names.Revert (RevertName (..), decodeRevert)
 import System.Timeout (timeout)
 
 data ChainClient = ChainClient
@@ -227,7 +227,7 @@ rpc ChainClient {ccManager, ccConfig = ChainConfig {cRpcUrl}, ccTimeout} method
       revertData <- e .:? "data"
       pure $
         if code == revertCode || "execution reverted" `T.isPrefixOf` message
-          then ChainReverted $ decodeRevert =<< (JT.parseMaybe bytes =<< revertData)
+          then ChainReverted $ maybe (Just RNWithoutData) (decodeRevert <=< JT.parseMaybe bytes) revertData
           else unavailable $ "error " <> tshow code <> ": " <> T.take maxNodeMessageLength message
     unavailable reason = ChainUnavailable $ method <> ": " <> reason
 

apps/simplex-store-service/src/StoreService/Names/Contracts.hs

@@ -41,6 +41,7 @@ import StoreService.Config (ChainConfig (..))
 import StoreService.Names.Abi
 import StoreService.Names.Chain (Block (..), ChainClient (..), ChainError (..), ethCall, latestBlock)
 import StoreService.Names.Registration (channelRecordKey, contactRecordKey, labelNode, nodeBytes)
+import StoreService.Names.Revert (RevertName (..))
 
 data Contracts = Contracts
   { ctChain :: ChainClient,
@@ -138,11 +139,12 @@ sponsorNonce ct owner = call ct (cSponsor $ chainConfig ct) ("nonces(address)",
 resolverNonce :: Contracts -> Address -> IO (Either ChainError Word64)
 resolverNonce ct owner = call ct (resolver ct) ("nonces(address)", addressWord owner) decodeWord64
 
--- | Nothing when nobody holds the name: @ownerOf@ reverts for it, and that revert is the answer.
+-- | Nothing when nobody holds the name: @ownerOf@ then fails the registrar's bare @require@ on the expiry,
+-- a revert without data. Any other revert is a failed read.
 ownerOfToken :: Contracts -> LabelHash -> IO (Either ChainError (Maybe Address))
 ownerOfToken ct lh =
   call ct (baseRegistrar ct) ("ownerOf(uint256)", labelHashArg lh) decodeAddress >>= \case
-    Left (ChainReverted _) -> pure $ Right Nothing
+    Left (ChainReverted (Just RNWithoutData)) -> pure $ Right Nothing
     r -> pure $ Just <$> r
 
 nameExpires :: Contracts -> LabelHash -> IO (Either ChainError Word64)

apps/simplex-store-service/src/StoreService/Names/Revert.hs

@@ -55,59 +55,65 @@ data RevertName
   | RNTransferToSelf
   | RNUnexpiredCommitmentExists
   | RNZeroAddress
+  | -- | a revert with empty or no data, as a bare @require@ or @revert()@ gives
+    RNWithoutData
   deriving (Eq, Show, Enum, Bounded)
 
-revertSignature :: RevertName -> ByteString
+-- | Nothing for a revert without data, which has no selector.
+revertSignature :: RevertName -> Maybe ByteString
 revertSignature = \case
-  RNAlreadyFrozen -> "AlreadyFrozen()"
-  RNCommitmentNotFound -> "CommitmentNotFound(bytes32)"
-  RNCommitmentTooNew -> "CommitmentTooNew(bytes32,uint256,uint256)"
-  RNCommitmentTooOld -> "CommitmentTooOld(bytes32,uint256,uint256)"
-  RNDurationTooShort -> "DurationTooShort(uint256)"
-  RNFrozen -> "Frozen()"
-  RNInsufficientAllowance -> "InsufficientAllowance(uint256,uint256)"
-  RNInsufficientValue -> "InsufficientValue()"
-  RNInvalidEVMAddress -> "InvalidEVMAddress(bytes)"
-  RNInvalidNonce -> "InvalidNonce()"
-  RNInvalidSignature -> "InvalidSignature()"
-  RNLabelTooLong -> "LabelTooLong(uint256,uint256)"
-  RNMaxCommitmentAgeTooHigh -> "MaxCommitmentAgeTooHigh()"
-  RNMaxCommitmentAgeTooLow -> "MaxCommitmentAgeTooLow()"
-  RNMinCharLengthCanOnlyDecrease -> "MinCharLengthCanOnlyDecrease()"
-  RNMinCharLengthZero -> "MinCharLengthZero()"
-  RNMinCommitmentAgeZero -> "MinCommitmentAgeZero()"
-  RNNameNotAvailable -> "NameNotAvailable(string)"
-  RNNameNotReserved -> "NameNotReserved(string)"
-  RNNameReserved -> "NameReserved(string)"
-  RNNameTooShort -> "NameTooShort(string,uint8)"
-  RNNftGateCanOnlyBeDisabled -> "NftGateCanOnlyBeDisabled()"
-  RNNftRequired -> "NftRequired()"
-  RNNodeNotOwnedByRegistrar -> "NodeNotOwnedByRegistrar()"
-  RNNotBeneficiary -> "NotBeneficiary()"
-  RNNotNameOwner -> "NotNameOwner()"
-  RNNotOffered -> "NotOffered()"
-  RNNotOwnerOrBeneficiary -> "NotOwnerOrBeneficiary()"
-  RNNotSponsor -> "NotSponsor()"
-  RNNotSubnameRegistrar -> "NotSubnameRegistrar()"
-  RNOffsetOutOfBoundsError -> "OffsetOutOfBoundsError(uint256,uint256)"
-  RNOwnerMustBeCaller -> "OwnerMustBeCaller()"
-  RNPublicSalesClosed -> "PublicSalesClosed()"
-  RNReasonRequired -> "ReasonRequired()"
-  RNRecipientMustBeCaller -> "RecipientMustBeCaller()"
-  RNReentrantCall -> "ReentrantCall()"
-  RNResolverRequiredWhenDataSupplied -> "ResolverRequiredWhenDataSupplied()"
-  RNReverseRecordNotSupported -> "ReverseRecordNotSupported()"
-  RNSignatureExpired -> "SignatureExpired()"
-  RNTransferFailed -> "TransferFailed()"
-  RNTransferToSelf -> "TransferToSelf()"
-  RNUnexpiredCommitmentExists -> "UnexpiredCommitmentExists(bytes32)"
-  RNZeroAddress -> "ZeroAddress()"
+  RNAlreadyFrozen -> Just "AlreadyFrozen()"
+  RNCommitmentNotFound -> Just "CommitmentNotFound(bytes32)"
+  RNCommitmentTooNew -> Just "CommitmentTooNew(bytes32,uint256,uint256)"
+  RNCommitmentTooOld -> Just "CommitmentTooOld(bytes32,uint256,uint256)"
+  RNDurationTooShort -> Just "DurationTooShort(uint256)"
+  RNFrozen -> Just "Frozen()"
+  RNInsufficientAllowance -> Just "InsufficientAllowance(uint256,uint256)"
+  RNInsufficientValue -> Just "InsufficientValue()"
+  RNInvalidEVMAddress -> Just "InvalidEVMAddress(bytes)"
+  RNInvalidNonce -> Just "InvalidNonce()"
+  RNInvalidSignature -> Just "InvalidSignature()"
+  RNLabelTooLong -> Just "LabelTooLong(uint256,uint256)"
+  RNMaxCommitmentAgeTooHigh -> Just "MaxCommitmentAgeTooHigh()"
+  RNMaxCommitmentAgeTooLow -> Just "MaxCommitmentAgeTooLow()"
+  RNMinCharLengthCanOnlyDecrease -> Just "MinCharLengthCanOnlyDecrease()"
+  RNMinCharLengthZero -> Just "MinCharLengthZero()"
+  RNMinCommitmentAgeZero -> Just "MinCommitmentAgeZero()"
+  RNNameNotAvailable -> Just "NameNotAvailable(string)"
+  RNNameNotReserved -> Just "NameNotReserved(string)"
+  RNNameReserved -> Just "NameReserved(string)"
+  RNNameTooShort -> Just "NameTooShort(string,uint8)"
+  RNNftGateCanOnlyBeDisabled -> Just "NftGateCanOnlyBeDisabled()"
+  RNNftRequired -> Just "NftRequired()"
+  RNNodeNotOwnedByRegistrar -> Just "NodeNotOwnedByRegistrar()"
+  RNNotBeneficiary -> Just "NotBeneficiary()"
+  RNNotNameOwner -> Just "NotNameOwner()"
+  RNNotOffered -> Just "NotOffered()"
+  RNNotOwnerOrBeneficiary -> Just "NotOwnerOrBeneficiary()"
+  RNNotSponsor -> Just "NotSponsor()"
+  RNNotSubnameRegistrar -> Just "NotSubnameRegistrar()"
+  RNOffsetOutOfBoundsError -> Just "OffsetOutOfBoundsError(uint256,uint256)"
+  RNOwnerMustBeCaller -> Just "OwnerMustBeCaller()"
+  RNPublicSalesClosed -> Just "PublicSalesClosed()"
+  RNReasonRequired -> Just "ReasonRequired()"
+  RNRecipientMustBeCaller -> Just "RecipientMustBeCaller()"
+  RNReentrantCall -> Just "ReentrantCall()"
+  RNResolverRequiredWhenDataSupplied -> Just "ResolverRequiredWhenDataSupplied()"
+  RNReverseRecordNotSupported -> Just "ReverseRecordNotSupported()"
+  RNSignatureExpired -> Just "SignatureExpired()"
+  RNTransferFailed -> Just "TransferFailed()"
+  RNTransferToSelf -> Just "TransferToSelf()"
+  RNUnexpiredCommitmentExists -> Just "UnexpiredCommitmentExists(bytes32)"
+  RNZeroAddress -> Just "ZeroAddress()"
+  RNWithoutData -> Nothing
 
 revertSelectors :: Map ByteString RevertName
-revertSelectors = M.fromList [(selector $ revertSignature n, n) | n <- [minBound .. maxBound]]
+revertSelectors = M.fromList [(selector signature, n) | n <- [minBound .. maxBound], Just signature <- [revertSignature n]]
 
 -- | The error a revert's data names by its selector, if it is one of the contracts' errors.
 decodeRevert :: ByteString -> Maybe RevertName
-decodeRevert revertData = M.lookup (B.take selectorSize revertData) revertSelectors
+decodeRevert revertData
+  | B.null revertData = Just RNWithoutData
+  | otherwise = M.lookup (B.take selectorSize revertData) revertSelectors
   where
     selectorSize = 4

tests/Bots/StoreService/NamesChainTests.hs

@@ -31,7 +31,7 @@ namesChainTests = describe "StoreService names: chain" $ do
     it "reads a dry run's revert with data by its name" $ testDryRunRevertNamed g
     it "reads an eth_call revert with data by its name" $ testCallRevertNamed g
     it "reads ownerOf of a name nobody holds as a revert" $ testOwnerOfRevert g
-    it "reads a revert without code 3 or data as a revert" $ testRevertWithoutData g
+    it "reads a revert without code 3 or data as a revert without data" $ testRevertWithoutData g
     it "reads a revert of an unknown error as an unnamed revert" $ testUnknownRevert g
   describe "failures" $ do
     it "reads an RPC error as unavailable, without the URL" $ testRpcError g
@@ -101,9 +101,10 @@ viemSelectors =
 
 testRevertSelectors :: IO ()
 testRevertSelectors = do
-  map snd viemSelectors `shouldBe` [minBound .. maxBound]
+  map snd viemSelectors `shouldBe` filter (/= RNWithoutData) [minBound .. maxBound]
   [(n, decodeRevert $ unhex sel <> uint256 (0 :: Int)) | (sel, n) <- viemSelectors] `shouldBe` [(n, Just n) | (_, n) <- viemSelectors]
   decodeRevert (B.take 3 $ unhex "74480cc9") `shouldBe` Nothing
+  decodeRevert "" `shouldBe` Just RNWithoutData
 
 testDryRunRevertNamed :: TVar ChaChaDRG -> IO ()
 testDryRunRevertNamed g = withChain g $ \node c -> do
@@ -120,12 +121,12 @@ testOwnerOfRevert :: TVar ChaChaDRG -> IO ()
 testOwnerOfRevert g = withChain g $ \node c@ChainClient {ccConfig = ChainConfig {cBaseRegistrar}} -> do
   let ownerOf = selector "ownerOf(uint256)" <> C.keccak256 "bakery1"
   answerCall node cBaseRegistrar ownerOf $ Reverts ""
-  ethCall c cBaseRegistrar ownerOf `shouldReturn` Left (ChainReverted Nothing)
+  ethCall c cBaseRegistrar ownerOf `shouldReturn` Left (ChainReverted $ Just RNWithoutData)
 
 testRevertWithoutData :: TVar ChaChaDRG -> IO ()
 testRevertWithoutData g = withChain g $ \node c -> do
   setFault node "eth_call" $ RpcError (-32000) "execution reverted" Nothing
-  ethCall c fakeController availableCall `shouldReturn` Left (ChainReverted Nothing)
+  ethCall c fakeController availableCall `shouldReturn` Left (ChainReverted $ Just RNWithoutData)
 
 testUnknownRevert :: TVar ChaChaDRG -> IO ()
 testUnknownRevert g = withChain g $ \node c -> do

tests/Bots/StoreService/NamesContractsTests.hs

@@ -20,6 +20,7 @@ import StoreService.Names.Abi
 import StoreService.Names.Chain
 import StoreService.Names.Contracts
 import StoreService.Names.Registration (labelNode, namehash, nodeBytes)
+import StoreService.Names.Revert (RevertName (..))
 import Test.Hspec
 
 namesContractsTests :: Spec
@@ -44,7 +45,8 @@ namesContractsTests = describe "StoreService names: contracts" $ do
     it "reads the oracle from the controller on each call" $ testOracleReplaced g
   describe "other reads" $ do
     it "reads the sponsor's and the resolver's nonces from their own contracts" $ testNonces g
-    it "reads ownerOf's revert as nobody, and a failed read as a failure" $ testOwnerOf g
+    it "reads ownerOf's revert without data as nobody" $ testOwnerOfNobody g
+    it "reads any other ownerOf revert or failure as a failed read" $ testOwnerOfFailed g
     it "reads a commitment's time, none as Nothing" $ testCommitmentTime g
     it "reads the controller's availability and the name's expiry" $ testAvailability g
     it "reads the commitment ages once" $ testCommitmentAges g
@@ -161,12 +163,24 @@ testNonces g = withContracts g $ \node ct -> do
   sponsorNonce ct owner `shouldReturn` Right 5
   resolverNonce ct owner `shouldReturn` Right 3
 
-testOwnerOf :: TVar ChaChaDRG -> IO ()
-testOwnerOf g = withContracts g $ \node ct -> do
+testOwnerOfNobody :: TVar ChaChaDRG -> IO ()
+testOwnerOfNobody g = withContracts g $ \node ct -> do
   answerCall node (registrarOf node) ownerOfCall $ Returns $ addressWord owner
   ownerOfToken ct alice `shouldReturn` Right (Just owner)
+  -- the registrar's require(expiries[tokenId] > block.timestamp), as anvil answers it with "0x" and reth without data
   answerCall node (registrarOf node) ownerOfCall $ Reverts ""
   ownerOfToken ct alice `shouldReturn` Right Nothing
+  setFault node "eth_call" $ RpcError 3 "execution reverted" Nothing
+  ownerOfToken ct alice `shouldReturn` Right Nothing
+
+testOwnerOfFailed :: TVar ChaChaDRG -> IO ()
+testOwnerOfFailed g = withContracts g $ \node ct -> do
+  -- viem's encodeErrorResult of Error("ERC721: invalid token ID")
+  answerCall node (registrarOf node) ownerOfCall $ Reverts $ unhex "08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000184552433732313a20696e76616c696420746f6b656e2049440000000000000000"
+  ownerOfToken ct alice `shouldReturn` Left (ChainReverted Nothing)
+  -- viem's encodeErrorResult of ReentrantCall()
+  answerCall node (registrarOf node) ownerOfCall $ Reverts $ unhex "37ed32e8"
+  ownerOfToken ct alice `shouldReturn` Left (ChainReverted $ Just RNReentrantCall)
   setFault node "eth_call" HttpFailure
   ownerOfToken ct alice `shouldReturn` Left (ChainUnavailable "eth_call: HTTP 502")
 
@@ -200,13 +214,13 @@ testFailedRead g = withContracts g $ \node ct -> do
   setBlock node 100 now
   answerCall node fakeController reservedNamesCall $ Returns $ uint256 (0 :: Int)
   answerCall node (registrarOf node) nameExpiresCall $ Reverts ""
-  nameStatus ct alice `shouldReturn` Left (ChainReverted Nothing)
+  nameStatus ct alice `shouldReturn` Left (ChainReverted $ Just RNWithoutData)
   answerCall node (registrarOf node) nameExpiresCall $ Returns $ uint256 (0 :: Int)
   setFault node "eth_getBlockByNumber" HttpFailure
   nameStatus ct alice `shouldReturn` Left (ChainUnavailable "eth_getBlockByNumber: HTTP 502")
   clearFault node "eth_getBlockByNumber"
   answerCall node fakeController reservedNamesCall $ Reverts ""
-  nameStatus ct alice `shouldReturn` Left (ChainReverted Nothing)
+  nameStatus ct alice `shouldReturn` Left (ChainReverted $ Just RNWithoutData)
 
 testMalformed :: TVar ChaChaDRG -> IO ()
 testMalformed g = withContracts g $ \node ct -> do