Fix name resolution within module redeploy

docs/en/pact-functions.md

@@ -1928,7 +1928,7 @@ Retreive any accumulated events and optionally clear event state. Object returne
  *→* `[string]`
 
 
-Queries, or with arguments, sets execution config flags. Valid flags: ["AllowReadInLocal","DisableHistoryInTransactionalMode","DisableInlineMemCheck","DisableModuleInstall","DisableNewTrans","DisablePact40","DisablePact420","DisablePact43","DisablePact431","DisablePact44","DisablePact45","DisablePact46","DisablePact47","DisablePactEvents","DisableRuntimeReturnTypeChecking","EnforceKeyFormats","OldReadOnlyBehavior","PreserveModuleIfacesBug","PreserveModuleNameBug","PreserveNsModuleInstallBug","PreserveShowDefs"]
+Queries, or with arguments, sets execution config flags. Valid flags: ["AllowReadInLocal","DisableHistoryInTransactionalMode","DisableInlineMemCheck","DisableModuleInstall","DisableNewTrans","DisablePact40","DisablePact420","DisablePact43","DisablePact431","DisablePact44","DisablePact45","DisablePact46","DisablePact47","DisablePact48","DisablePactEvents","DisableRuntimeReturnTypeChecking","EnforceKeyFormats","OldReadOnlyBehavior","PreserveModuleIfacesBug","PreserveModuleNameBug","PreserveNsModuleInstallBug","PreserveShowDefs"]
 ```lisp
 pact> (env-exec-config ['DisableHistoryInTransactionalMode]) (env-exec-config)
 ["DisableHistoryInTransactionalMode"]

src/Pact/Eval.hs

@@ -89,6 +89,7 @@ import Pact.Types.Purity
 import Pact.Types.Runtime
 import Pact.Types.SizeOf
 import Pact.Types.Namespace
+import Control.Applicative (liftA2)
 
 
 evalBeginTx :: Info -> Eval e (Maybe TxId)
@@ -320,6 +321,11 @@ eval' (TModule _tm@(MDModule m) bod i) =
         capMName <-
           ifExecutionFlagSet' FlagPreserveNsModuleInstallBug (_mName m) (_mName mangledM)
         void $ acquireModuleAdminCapability capMName $ return ()
+
+        unlessExecutionFlagSet FlagDisablePact48 $ do
+          evalRefs.rsLoadedModules %= HM.delete (_mName mangledM)
+          evalRefs.rsQualifiedDeps %= HM.filterWithKey (\k _ -> _fqModule k /= _mName mangledM)
+
     -- build/install module from defs
     (g,govM) <- loadModule mangledM bod i g0
     szVer <- getSizeOfVersion
@@ -737,27 +743,42 @@ fullyQualifyDefs info mdef defs = do
     checkAddDep = \case
       Direct (TVar (FQName fq) _) -> modify' (Set.insert (_fqModule fq))
       _ -> pure ()
-    -- | traverse to find deps and form graph
-    traverseGraph allDefs memo = fmap stronglyConnCompR $ forM (HM.toList allDefs) $ \(defName,defTerm) -> do
-      let defName' = FullyQualifiedName defName (_mName mdef) (moduleHash mdef)
-      defTerm' <- forM defTerm $ \(f :: Name) -> do
+
+    resolveBareName memo f@(BareName fn _) = case HM.lookup fn defs of
+      Just _ -> do
+        let name' = FullyQualifiedName fn (_mName mdef) (moduleHash mdef)
+        return (Left name') -- decl found
+      Nothing -> lift (resolveBareModRef info f fn memo (MDModule mdef)) >>= \case
+        Just mr -> return (Right mr) -- mod ref found
+        Nothing -> resolveError f
+
+    resolveError f = lift (evalError' f $ "Cannot resolve " <> dquotes (pretty f))
+
+    resolveName flagPact48Disabled memo = \case
+      (QName (QualifiedName (ModuleName mn mNs) fn i))
+        | not flagPact48Disabled
+          && mn == _mnName (_mName mdef)
+          && isNsMatch -> resolveBareName memo (BareName fn i)
+          where
+            isNsMatch = fromMaybe True (liftA2 (==) modNs mNs)
+            modNs = _mnNamespace (_mName mdef)
+      f  -> do
         dm <- lift (resolveRefFQN f f) -- lookup ref, don't try modules for barenames
         case (dm, f) of
           (Just t, _) -> checkAddDep t *> return (Right t) -- ref found
-          -- for barenames, check decls and finally modules
-          (Nothing, Name (BareName fn _)) ->
-            case HM.lookup fn allDefs of
-              Just _ -> do
-                let name' = FullyQualifiedName fn (_mName mdef) (moduleHash mdef)
-                return (Left name') -- decl found
-              Nothing -> lift (resolveBareModRef info f fn memo (MDModule mdef)) >>= \r -> case r of
-                Just mr -> return (Right mr) -- mod ref found
-                Nothing ->
-                  lift (evalError' f $ "Cannot resolve " <> dquotes (pretty f))
-          -- for qualified names, simply fail
-          (Nothing, _) -> lift (evalError' f $ "Cannot resolve " <> dquotes (pretty f))
+            -- for barenames, check decls and finally modules
+          (Nothing, Name bn@BareName{}) -> resolveBareName memo bn
+              -- for qualified names, simply fail
+          (Nothing, _) -> resolveError f
+
+    -- | traverse to find deps and form graph
+    traverseGraph allDefs memo = fmap stronglyConnCompR $ forM (HM.toList allDefs) $ \(defName,defTerm) -> do
+      let defName' = FullyQualifiedName defName (_mName mdef) (moduleHash mdef)
+      disablePact48 <- lift (isExecutionFlagSet FlagDisablePact48)
+      defTerm' <- forM defTerm $ \(f :: Name) -> resolveName disablePact48 memo f
 
       return (defTerm', defName', mapMaybe (either Just (const Nothing)) $ toList defTerm')
+
     moduleHash = _mhHash . _mHash
 
 

src/Pact/Types/Runtime.hs

@@ -189,6 +189,8 @@ data ExecutionFlag
   | FlagDisablePact47
   -- | Disable runtime return type checking.
   | FlagDisableRuntimeReturnTypeChecking
+  -- | Disable Pact 4.8 Features
+  | FlagDisablePact48
   deriving (Eq,Ord,Show,Enum,Bounded)
 
 -- | Flag string representation

tests/pact/fqns.repl

@@ -77,3 +77,165 @@
 
 (expect "selects correct test" (modB.get-test) "hello")
 (commit-tx)
+
+;;
+;; Module redeploy name resolution
+;;
+
+; In the following tests, we define a module `test-mod-redeploy-ref`, and then
+; redeploy the same module with the change to one capability: `test`.
+; In the old version, the `test` capability fails, in the new one it passes.
+
+(begin-tx)
+; First, demonstrate the behavior prior to pact-4.8.
+(env-exec-config ["DisablePact48"])
+
+(namespace 'free)
+(module test-mod-redeploy-ref g
+  (defcap g () true)
+
+  (defcap test ()
+    (enforce false "boom"))
+
+  (defun f ()
+    (with-capability (test)
+      1))
+  )
+; Before pact-4.8, the updated capability will be ignored, and calls to a function
+; requiring that capability will fail. This is true whether we invoke `f` with or
+; without its module name, and with or without its namespace.
+(expect-failure "Execution expected to fail due to assertion failure \
+                \within defcap invoke - invoking with an unqualified reference." (f))
+
+(expect-failure "Execution expected to fail due to assertion failure \
+                \within defcap invoke - invoking with an qualified reference \
+                \(no namespace)." (test-mod-redeploy-ref.f))
+
+(expect-failure "Execution expected to fail due to assertion failure within \
+                \defcap invoke - invoking with an fully qualified reference."
+                (free.test-mod-redeploy-ref.f))
+(commit-tx)
+
+; The following module redeployment changed the capability `test` to pass.
+(begin-tx)
+(namespace 'free)
+(module test-mod-redeploy-ref g
+  (defcap g () true)
+  (defcap test ()
+    true)
+
+  (defun f ()
+    (with-capability (free.test-mod-redeploy-ref.test)
+      1))
+
+  (defun f1 ()
+    (with-capability (test-mod-redeploy-ref.test)
+      1))
+
+  )
+; Before pact-4.8, the capability update (passing `test`) was ignored as the
+; full-qualified reference referenced the previously deployed version of the module.
+(expect-failure "Execution expected to fail because `DisablePact48` will reference \
+                \the prev. deployed version (fully qualified capability) \
+                \- invoking with an unqualified reference." (f))
+
+(expect-failure "Execution expected to fail because `DisablePact48` will reference \
+                \the prev. deployed version (fully qualified capability) \
+                \- invoking with an qualified reference (no namespace)."
+                (test-mod-redeploy-ref.f))
+
+(expect-failure "Execution expected to fail because `DisablePact48` will reference \
+                \the prev. deployed version (fully qualified capability) \
+                \- invoking with a fully qualified reference."
+                (free.test-mod-redeploy-ref.f))
+
+; We repeat almost the same tests but without prefixing the namespace.
+(expect-failure "Execution expected to fail because `DisablePact48` will reference \
+                \the prev. deployed version (qualified capability without namespace) \
+                \- invoking with an unqualified reference."
+                (f1))
+
+(expect-failure "Execution expected to fail because `DisablePact48` will reference \
+                \the prev. deployed version (qualified capability without namespace) \
+                \- invoking with an qualified reference (no namespace)."
+                (test-mod-redeploy-ref.f1))
+
+(expect-failure "Execution expected to fail because `DisablePact48` will reference \
+                \the prev. deployed version (qualified capability without namespace) \
+                \- invoking with a fully qualified reference."
+                (free.test-mod-redeploy-ref.f1))
+(commit-tx)
+
+
+;; Check Pact48 behaviour
+
+; The pact-4.8 update uses the new module reference.
+(begin-tx)
+(env-exec-config []) ; reset
+(namespace 'free)
+(module test-mod-redeploy-ref g
+  (defcap g () true)
+
+  (defcap test ()
+    (enforce false "boom"))
+
+  (defun f ()
+    (with-capability (test)
+      1))
+  )
+; These tests show that failing capabilities work as expected.
+; Later we will do a module redeploy to illustrate the (interesting) new behavior.
+(expect-failure "Execution expected to fail due to assertion failure \
+                \within defcap invoke - invoking with an unqualified reference." (f))
+
+(expect-failure "Execution expected to fail due to assertion failure \
+                \within defcap invoke - invoking with an qualified reference \
+                \(no namespace)." (test-mod-redeploy-ref.f))
+
+(expect-failure "Execution expected to fail due to assertion failure within \
+                \defcap invoke - invoking with an fully qualified reference."
+                (free.test-mod-redeploy-ref.f))
+
+(commit-tx)
+
+(begin-tx)
+(namespace 'free)
+(module test-mod-redeploy-ref g
+  (defcap g () true)
+  (defcap test ()
+    true)
+  (defun f ()
+    (with-capability (free.test-mod-redeploy-ref.test)
+      1))
+
+  (defun f1 ()
+    (with-capability (test-mod-redeploy-ref.test)
+      1))
+  )
+; These tests show that f now references the updated version of the fully qualified capability,
+; regardless of whether f is invoked with its namespace and with its module name.
+(expect "Executation expected to find reference (fully qualified) to the current module \
+        \(fully qualified reference) - invoking with an unqualified reference."
+        1 (f))
+
+(expect "Executation expected to find reference (fully qualified) to the current module \
+        \(fully qualified reference) - invoking with an qualified reference (no namespace)."
+        1 (test-mod-redeploy-ref.f))
+
+(expect "Executation expected to find reference (fully qualified) to the current module \
+        \(fully qualified reference) - invoking with a fully qualified reference."
+        1 (free.test-mod-redeploy-ref.f))
+
+; The function should see the updated capability even if it used references without namespace.
+(expect "Executation expected to find reference (qualified, no namespace) to the current module \
+        \(fully qualified reference) - invoking with an unqualified reference."
+        1 (f1))
+
+(expect "Executation expected to find reference (qualified, no namespace) to the current module \
+        \(fully qualified reference) - invoking with an qualified reference (no namespace)."
+        1 (test-mod-redeploy-ref.f1))
+
+(expect "Executation expected to find reference (qualified, no namespace) to the current module \
+        \(fully qualified reference) - invoking with a fully qualified reference."
+        1 (free.test-mod-redeploy-ref.f1))
+(commit-tx)