fix: root contract with unsafeClient should throw even with unsafeCli…

…ent = skip

src/__tests__/integration/basic/evolve-unsafe-contract.test.ts

@@ -6,7 +6,6 @@ import { JWKInterface } from 'arweave/node/lib/wallet';
 import path from 'path';
 import { mineBlock } from '../_helpers';
 import { PstState, PstContract } from '../../../contract/PstContract';
-import { InteractionResult } from '../../../core/modules/impl/HandlerExecutorFactory';
 import { Warp } from '../../../core/Warp';
 import { WarpFactory } from '../../../core/WarpFactory';
 import { LoggerFactory } from '../../../logging/LoggerFactory';
@@ -111,26 +110,16 @@ describe('Testing unsafe client in nested contracts with "skip" option', () => {
       qty: 555
     });
     await mineBlock(warp);
-    // note: at this point we have already cached state until the 'evolve' and no more interactions
-    // should be evaluated
-    const result = await pst.readState();
 
-    // note: should not evaluate at all the last interaction
-    expect(Object.keys(result.cachedValue.validity).length).toEqual(2);
-    expect(Object.keys(result.cachedValue.errorMessages).length).toEqual(1);
-
-    expect(result.cachedValue.validity[evolveResponse.originalTxId]).toBe(false);
-    expect(result.cachedValue.errorMessages[evolveResponse.originalTxId]).toMatch(
-      'Skipping evaluation of the unsafe contract'
-    );
+    // even with 'skip', the unsafe client on root contract will throw
+    await expect(pst.readState()).rejects.toThrow('[SkipUnsafeError]');
 
     // testcase for new warp instance
     const newWarp = WarpFactory.forLocal(1668);
     const freshPst = newWarp.contract(contractTxId).setEvaluationOptions({
       unsafeClient: 'allow'
     });
     const freshResult = await freshPst.readState();
-    console.log(freshResult.cachedValue.validity);
     // note: should not evaluate at all the last interaction
     expect(Object.keys(freshResult.cachedValue.validity).length).toEqual(4);
     expect(Object.keys(freshResult.cachedValue.errorMessages).length).toEqual(0);

src/__tests__/integration/basic/unsafe-contract.test.ts

@@ -81,7 +81,7 @@ describe('Testing the Warp client', () => {
 
   it('should not allow to evaluate contract with unsafe operations by default', async () => {
     await expect(contract.readState()).rejects.toThrowError(
-      '[SkipUnsafeError] Using unsafeClient is not allowed by default. Use EvaluationOptions.allowUnsafeClient flag'
+      '[SkipUnsafeError]'
     );
   });
 
@@ -97,17 +97,14 @@ describe('Testing the Warp client', () => {
     });
     await mineBlock(warp);
 
-    const result = await contractWithUnsafeSkip.readState();
-    expect(result.cachedValue.state).toEqual(initialState);
-    expect(result.sortKey).toEqual(genesisSortKey);
+    // even with 'skip', the unsafe client on root contract will throw
+    await expect(contractWithUnsafeSkip.readState()).rejects.toThrow('[SkipUnsafeError]');
 
     // fresh warp instance - skip
     const newWarp = WarpFactory.forLocal(1801);
     const freshPst = newWarp.contract(contractTxId).setEvaluationOptions({
       unsafeClient: 'skip'
     });
-    const freshResult = await freshPst.readState();
-    expect(freshResult.cachedValue.state).toEqual(initialState);
-    expect(freshResult.sortKey).toEqual(genesisSortKey);
+    await expect(freshPst.readState()).rejects.toThrow('[SkipUnsafeError]');
   });
 });

src/contract/HandlerBasedContract.ts

@@ -5,33 +5,32 @@ import { SortKeyCacheResult } from '../cache/SortKeyCache';
 import { ContractCallRecord, InteractionCall } from '../core/ContractCallRecord';
 import { ExecutionContext } from '../core/ExecutionContext';
 import {
-  InteractionResult,
-  HandlerApi,
   ContractInteraction,
+  HandlerApi,
   InteractionData,
-  ContractError
+  InteractionResult
 } from '../core/modules/impl/HandlerExecutorFactory';
 import { LexicographicalInteractionsSorter } from '../core/modules/impl/LexicographicalInteractionsSorter';
 import { InteractionsSorter } from '../core/modules/InteractionsSorter';
-import { EvaluationOptions, DefaultEvaluationOptions, EvalStateResult } from '../core/modules/StateEvaluator';
+import { DefaultEvaluationOptions, EvalStateResult, EvaluationOptions } from '../core/modules/StateEvaluator';
 import { SmartWeaveTags } from '../core/SmartWeaveTags';
 import { Warp } from '../core/Warp';
-import { createInteractionTx, createDummyTx } from '../legacy/create-interaction-tx';
+import { createDummyTx, createInteractionTx } from '../legacy/create-interaction-tx';
 import { GQLNodeInterface } from '../legacy/gqlResult';
 import { Benchmark } from '../logging/Benchmark';
 import { LoggerFactory } from '../logging/LoggerFactory';
 import { Evolve } from '../plugins/Evolve';
 import { ArweaveWrapper } from '../utils/ArweaveWrapper';
 import { sleep } from '../utils/utils';
 import {
-  Contract,
   BenchmarkStats,
+  Contract,
   CurrentTx,
+  InnerCallData,
   WriteInteractionOptions,
-  WriteInteractionResponse,
-  InnerCallData
+  WriteInteractionResponse
 } from './Contract';
-import { Tags, ArTransfer, emptyTransfer, ArWallet } from './deploy/CreateContract';
+import { ArTransfer, ArWallet, emptyTransfer, Tags } from './deploy/CreateContract';
 import { InnerWritesEvaluator } from './InnerWritesEvaluator';
 import { generateMockVrf } from '../utils/vrf';
 import { Signature, SignatureType } from './Signature';
@@ -514,23 +513,7 @@ export class HandlerBasedContract<State> implements Contract<State> {
 
   private async safeGetHandler(contractDefinition: ContractDefinition<any>): Promise<HandlerApi<State> | null> {
     const { executorFactory } = this.warp;
-    try {
-      return (await executorFactory.create(
-        contractDefinition,
-        this._evaluationOptions,
-        this.warp
-      )) as HandlerApi<State>;
-    } catch (e) {
-      if (e.name == 'ContractError' && e.subtype == 'unsafeClientSkip') {
-        if (this.isRoot()) {
-          return null;
-        } else {
-          throw new ContractError(e.message, e.subtype);
-        }
-      } else {
-        throw e;
-      }
-    }
+    return (await executorFactory.create(contractDefinition, this._evaluationOptions, this.warp)) as HandlerApi<State>;
   }
 
   private getToSortKey(upToSortKey?: string) {

src/core/modules/impl/CacheableStateEvaluator.ts

@@ -90,16 +90,6 @@ export class CacheableStateEvaluator extends DefaultStateEvaluator {
     const baseValidity = cachedState == null ? {} : cachedState.cachedValue.validity;
     const baseErrorMessages = cachedState == null ? {} : cachedState.cachedValue.errorMessages;
 
-    this.cLogger.debug('Base state', baseState);
-
-    if (executionContext.handler == null) {
-      // nothing to do - null was set by the 'createExecutionContext', so we're returning immediately
-      return new SortKeyCacheResult<EvalStateResult<State>>(
-        cachedState == null ? genesisSortKey : cachedState.sortKey,
-        new EvalStateResult(baseState, baseValidity, baseErrorMessages || {})
-      );
-    }
-
     // eval state for the missing transactions - starting from the latest value from cache.
     return await this.doReadState(
       missingInteractions,

src/core/modules/impl/HandlerExecutorFactory.ts

@@ -167,7 +167,7 @@ export class HandlerExecutorFactory implements ExecutorFactory<HandlerApi<unknow
           }
           case 'throw':
             throw new Error(
-              `[SkipUnsafeError] Using unsafeClient is not allowed by default. Use EvaluationOptions.allowUnsafeClient flag to evaluate ${contractDefinition.txId}.`
+              `[SkipUnsafeError] Using unsafeClient is not allowed by default. Use EvaluationOptions.unsafeClient flag to evaluate ${contractDefinition.txId}.`
             );
           case 'skip': {
             throw new ContractError(