diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 7060c3d7..a32de511 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -41,21 +41,21 @@ jobs: echo "VERSION=${BASE}-dev-${HASH}-${DATE}" echo "TAG=dev" echo "ENV=dev" - echo "LOG_LEVEL=all" + echo "LOG_LEVEL=6" } >> "$GITHUB_OUTPUT" elif [[ $ENV == "staging" ]]; then { echo "VERSION=${BASE}-staging" echo "TAG=staging" echo "ENV=staging" - echo "LOG_LEVEL=off" + echo "LOG_LEVEL=0" } >> "$GITHUB_OUTPUT" elif [[ $ENV == "production" ]]; then { echo "VERSION=${BASE}" echo "TAG=latest" echo "ENV=prod" - echo "LOG_LEVEL=off" + echo "LOG_LEVEL=0" } >> "$GITHUB_OUTPUT" else echo "Invalid environment" @@ -107,7 +107,7 @@ jobs: echo "Building UI with version $VERSION" - REACT_APP_DEBUG_LEVEL="${LOG_LEVEL}" REACT_APP_SNAP_VERSION="${VERSION}" yarn workspace wallet-ui build + REACT_APP_SNAP_VERSION="${VERSION}" yarn workspace wallet-ui build echo "Building Get Starknet with GET_STARKNET_PUBLIC_PATH=$GET_STARKNET_PUBLIC_PATH" diff --git a/packages/starknet-snap/.env.example b/packages/starknet-snap/.env.example index 43147264..c5b657e0 100644 --- a/packages/starknet-snap/.env.example +++ b/packages/starknet-snap/.env.example @@ -8,4 +8,9 @@ SNAP_ENV=dev VOYAGER_API_KEY= # Description: Environment variables for API key of ALCHEMY # Required: false -ALCHEMY_API_KEY= \ No newline at end of file +ALCHEMY_API_KEY= +# Description: Environment variables for Log Level, 0 does not log anything, 6 logs everything +# Possible Options: 0-6 +# Default: 0 +# Required: false +LOG_LEVEL=6 diff --git a/packages/starknet-snap/package.json b/packages/starknet-snap/package.json index 4fc5872f..4b589095 100644 --- a/packages/starknet-snap/package.json +++ b/packages/starknet-snap/package.json @@ -28,8 +28,8 @@ "serve": "mm-snap serve", "start": "mm-snap watch", "test": "yarn run test:unit && yarn run cover:report && yarn run jest", - "test:unit": "nyc --check-coverage --statements 50 --branches 50 --functions 50 --lines 50 mocha --colors -r ts-node/register \"test/**/*.test.ts\"", - "test:unit:one": "nyc --check-coverage --statements 50 --branches 50 --functions 50 --lines 50 mocha --colors -r ts-node/register" + "test:unit": "nyc --check-coverage --statements 50 --branches 50 --functions 50 --lines 50 mocha --colors -r ts-node/register --require test/global.ts \"test/**/*.test.ts\"", + "test:unit:one": "nyc --check-coverage --statements 50 --branches 50 --functions 50 --lines 50 mocha --colors -r ts-node/register --require test/global.ts" }, "nyc": { "exclude": [ diff --git a/packages/starknet-snap/snap.config.ts b/packages/starknet-snap/snap.config.ts index f8484fc3..056840ca 100644 --- a/packages/starknet-snap/snap.config.ts +++ b/packages/starknet-snap/snap.config.ts @@ -15,6 +15,7 @@ const config: SnapConfig = { SNAP_ENV: process.env.SNAP_ENV ?? 'prod', VOYAGER_API_KEY: process.env.VOYAGER_API_KEY ?? '', ALCHEMY_API_KEY: process.env.ALCHEMY_API_KEY ?? '', + LOG_LEVEL: process.env.LOG_LEVEL ?? '0', /* eslint-disable */ }, polyfills: true, diff --git a/packages/starknet-snap/src/config.ts b/packages/starknet-snap/src/config.ts new file mode 100644 index 00000000..16e80183 --- /dev/null +++ b/packages/starknet-snap/src/config.ts @@ -0,0 +1,10 @@ +import { LogLevel } from './utils'; + +export type SnapConfig = { + logLevel: string; +}; + +export const Config: SnapConfig = { + // eslint-disable-next-line no-restricted-globals + logLevel: process.env.LOG_LEVEL ?? LogLevel.OFF.valueOf().toString(), +}; diff --git a/packages/starknet-snap/src/index.test.ts b/packages/starknet-snap/src/index.test.ts index f6566cad..8ae24b5c 100644 --- a/packages/starknet-snap/src/index.test.ts +++ b/packages/starknet-snap/src/index.test.ts @@ -12,7 +12,6 @@ import { STARKNET_SEPOLIA_TESTNET_NETWORK, } from './utils/constants'; import * as keyPairUtils from './utils/keyPair'; -import { LogLevel, logger } from './utils/logger'; import * as starknetUtils from './utils/starknetUtils'; jest.mock('./utils/logger'); @@ -21,11 +20,9 @@ describe('onRpcRequest', () => { const createMockSpy = () => { const createAccountSpy = jest.spyOn(createAccountApi, 'createAccount'); const keyPairSpy = jest.spyOn(keyPairUtils, 'getAddressKeyDeriver'); - const getLogLevelSpy = jest.spyOn(logger, 'getLogLevel'); return { createAccountSpy, keyPairSpy, - getLogLevelSpy, }; }; @@ -42,11 +39,10 @@ describe('onRpcRequest', () => { }; it('processes request successfully', async () => { - const { createAccountSpy, keyPairSpy, getLogLevelSpy } = createMockSpy(); + const { createAccountSpy, keyPairSpy } = createMockSpy(); createAccountSpy.mockReturnThis(); keyPairSpy.mockReturnThis(); - getLogLevelSpy.mockReturnValue(LogLevel.OFF); await onRpcRequest(createMockRequest()); @@ -54,43 +50,16 @@ describe('onRpcRequest', () => { expect(createAccountSpy).toHaveBeenCalledTimes(1); }); - it('throws `Unable to execute the rpc request` error if an error has thrown and LogLevel is 0', async () => { - const { createAccountSpy, keyPairSpy, getLogLevelSpy } = createMockSpy(); + it('throws `Unable to execute the rpc request` error if an error has thrown', async () => { + const { createAccountSpy, keyPairSpy } = createMockSpy(); createAccountSpy.mockRejectedValue(new Error('Custom Error')); keyPairSpy.mockReturnThis(); - getLogLevelSpy.mockReturnValue(LogLevel.OFF); await expect(onRpcRequest(createMockRequest())).rejects.toThrow( 'Unable to execute the rpc request', ); }); - - it.each([ - LogLevel.DEBUG, - LogLevel.ALL, - LogLevel.ERROR, - LogLevel.INFO, - LogLevel.TRACE, - LogLevel.WARN, - ])( - `throws 'Unable to execute the rpc request' error if an error has thrown and LogLevel is %s`, - async function (logLevel) { - const { createAccountSpy, keyPairSpy, getLogLevelSpy } = createMockSpy(); - - createAccountSpy.mockRejectedValue(new Error('Custom Error')); - keyPairSpy.mockReturnThis(); - getLogLevelSpy.mockReturnValue(logLevel); - - await expect( - onRpcRequest( - createMockRequest({ - debugLevel: LogLevel[logLevel], - }), - ), - ).rejects.toThrow('Custom Error'); - }, - ); }); describe('onHomePage', () => { diff --git a/packages/starknet-snap/src/index.ts b/packages/starknet-snap/src/index.ts index 2d7e3d9e..1941bb49 100644 --- a/packages/starknet-snap/src/index.ts +++ b/packages/starknet-snap/src/index.ts @@ -17,6 +17,7 @@ import { ethers } from 'ethers'; import { addErc20Token } from './addErc20Token'; import { addNetwork } from './addNetwork'; +import { Config } from './config'; import { createAccount } from './createAccount'; import { declareContract } from './declareContract'; import { estimateAccDeployFee } from './estimateAccountDeployFee'; @@ -90,10 +91,8 @@ declare const snap; export const onRpcRequest: OnRpcRequestHandler = async ({ request }) => { const requestParams = request?.params as unknown as ApiRequestParams; - const debugLevel = requestParams?.debugLevel; - logger.init(debugLevel as unknown as string); - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - console.log(`debugLevel: ${logger.getLogLevel()}`); + + logger.logLevel = parseInt(Config.logLevel, 10); logger.log(`${request.method}:\nrequestParams: ${toJson(requestParams)}`); @@ -300,7 +299,7 @@ export const onRpcRequest: OnRpcRequestHandler = async ({ request }) => { // eslint-disable-next-line @typescript-eslint/restrict-template-expressions logger.error(`Error: ${error}`); // We don't want to expose the error message to the user when it is a production build - if (logger.getLogLevel() === LogLevel.OFF) { + if (logger.logLevel === LogLevel.OFF) { throw new SnapError( 'Unable to execute the rpc request', ) as unknown as Error; diff --git a/packages/starknet-snap/src/rpcs/displayPrivateKey.test.ts b/packages/starknet-snap/src/rpcs/displayPrivateKey.test.ts index f7ac8f0a..07c9faef 100644 --- a/packages/starknet-snap/src/rpcs/displayPrivateKey.test.ts +++ b/packages/starknet-snap/src/rpcs/displayPrivateKey.test.ts @@ -15,6 +15,8 @@ import { import { displayPrivateKey } from './displayPrivateKey'; import type { DisplayPrivateKeyParams } from './displayPrivateKey'; +jest.mock('../utils/logger'); + describe('displayPrivateKey', () => { const state: SnapState = { accContracts: [], diff --git a/packages/starknet-snap/src/rpcs/signTransaction.test.ts b/packages/starknet-snap/src/rpcs/signTransaction.test.ts index ca892fc0..2c7e1e14 100644 --- a/packages/starknet-snap/src/rpcs/signTransaction.test.ts +++ b/packages/starknet-snap/src/rpcs/signTransaction.test.ts @@ -18,6 +18,8 @@ import { import { signTransaction } from './signTransaction'; import type { SignTransactionParams } from './signTransaction'; +jest.mock('../utils/logger'); + describe('signTransaction', () => { const state: SnapState = { accContracts: [], diff --git a/packages/starknet-snap/src/types/snapApi.ts b/packages/starknet-snap/src/types/snapApi.ts index f3a69a65..87f3eccd 100644 --- a/packages/starknet-snap/src/types/snapApi.ts +++ b/packages/starknet-snap/src/types/snapApi.ts @@ -55,7 +55,6 @@ export type ApiRequestParams = export type BaseRequestParams = { chainId?: string; - debugLevel?: string; }; export type TransactionVersionParams = { diff --git a/packages/starknet-snap/src/utils/logger.test.ts b/packages/starknet-snap/src/utils/logger.test.ts new file mode 100644 index 00000000..7771fc8f --- /dev/null +++ b/packages/starknet-snap/src/utils/logger.test.ts @@ -0,0 +1,146 @@ +import { logger, LogLevel } from './logger'; + +describe('Logger', () => { + afterAll(() => { + logger.logLevel = LogLevel.OFF; + }); + const createLogSpy = () => { + return { + log: jest.spyOn(console, 'log').mockReturnThis(), + error: jest.spyOn(console, 'error').mockReturnThis(), + warn: jest.spyOn(console, 'warn').mockReturnThis(), + info: jest.spyOn(console, 'info').mockReturnThis(), + trace: jest.spyOn(console, 'trace').mockReturnThis(), + debug: jest.spyOn(console, 'debug').mockReturnThis(), + }; + }; + + const testLog = (message: string) => { + logger.log(message); + logger.error(message); + logger.warn(message); + logger.info(message); + logger.trace(message); + logger.debug(message); + }; + + it.each([ + { + logLevel: LogLevel.ALL, + expected: { + info: true, + warn: true, + error: true, + debug: true, + log: true, + trace: true, + }, + }, + { + logLevel: LogLevel.OFF, + expected: { + info: false, + warn: false, + error: false, + debug: false, + log: false, + trace: false, + }, + }, + { + logLevel: LogLevel.INFO, + expected: { + info: true, + warn: true, + error: true, + debug: false, + log: false, + trace: false, + }, + }, + { + logLevel: LogLevel.ERROR, + expected: { + info: false, + warn: false, + error: true, + debug: false, + log: false, + trace: false, + }, + }, + { + logLevel: LogLevel.WARN, + expected: { + info: false, + warn: true, + error: true, + debug: false, + log: false, + trace: false, + }, + }, + { + logLevel: LogLevel.DEBUG, + expected: { + info: true, + warn: true, + error: true, + debug: true, + log: false, + trace: false, + }, + }, + { + logLevel: LogLevel.TRACE, + expected: { + info: true, + warn: true, + error: true, + debug: true, + log: false, + trace: true, + }, + }, + ])( + 'logs correctly when `logLevel` is `$logLevel`', + ({ + logLevel, + expected, + }: { + logLevel: LogLevel; + expected: { + info: boolean; + warn: boolean; + error: boolean; + debug: boolean; + log: boolean; + trace: boolean; + }; + }) => { + const spys = createLogSpy(); + logger.logLevel = logLevel; + const logMsg = 'log'; + + testLog(logMsg); + + Object.entries(expected) + .filter(([_, value]) => !value) + .forEach(([key]) => { + expect(spys[key]).not.toHaveBeenCalled(); + }); + + Object.entries(expected) + .filter(([_, value]) => value) + .forEach(([key]) => { + expect(spys[key]).toHaveBeenCalledWith(logMsg); + }); + }, + ); + + it('return correct `LogLevel`', () => { + logger.logLevel = LogLevel.INFO; + + expect(logger.logLevel).toStrictEqual(LogLevel.INFO); + }); +}); diff --git a/packages/starknet-snap/src/utils/logger.ts b/packages/starknet-snap/src/utils/logger.ts index ba08fc15..538ba281 100644 --- a/packages/starknet-snap/src/utils/logger.ts +++ b/packages/starknet-snap/src/utils/logger.ts @@ -19,8 +19,8 @@ export type ILogger = { debug: LoggingFn; info: LoggingFn; trace: LoggingFn; - init: (level: string) => void; - getLogLevel: () => LogLevel; + init: () => void; + logLevel: LogLevel; }; export const emptyLog: LoggingFn = ( @@ -32,35 +32,31 @@ export const emptyLog: LoggingFn = ( // eslint-disable-next-line @typescript-eslint/no-empty-function {}; -class Logger implements ILogger { - readonly log: LoggingFn; +export class Logger implements ILogger { + log: LoggingFn; - readonly warn: LoggingFn; + warn: LoggingFn; - readonly error: LoggingFn; + error: LoggingFn; - readonly debug: LoggingFn; + debug: LoggingFn; - readonly info: LoggingFn; + info: LoggingFn; - readonly trace: LoggingFn; + trace: LoggingFn; - readonly #logLevel: LogLevel = LogLevel.OFF; + #logLevel: LogLevel = LogLevel.OFF; - constructor() { - this.init(LogLevel.OFF.toString()); + set logLevel(level: LogLevel) { + this.#logLevel = level; + this.init(); } - #setLogLevel = function (level: string): void { - if (level && Object.values(LogLevel).includes(level.toUpperCase())) { - this.#logLevel = LogLevel[level.toUpperCase()]; - } else { - this.#logLevel = LogLevel.OFF; - } - }; + get logLevel(): LogLevel { + return this.#logLevel; + } - public init = function (level: string): void { - this.#setLogLevel(level); + init(): void { this.error = console.error.bind(console); this.warn = console.warn.bind(console); this.info = console.info.bind(console); @@ -86,11 +82,7 @@ class Logger implements ILogger { if (this.#logLevel < LogLevel.ALL) { this.log = emptyLog; } - }; - - public getLogLevel = function (): LogLevel { - return this.#logLevel; - }; + } } export const logger = new Logger(); diff --git a/packages/starknet-snap/src/utils/superstruct.test.ts b/packages/starknet-snap/src/utils/superstruct.test.ts index 6e9bf450..ae05e1b7 100644 --- a/packages/starknet-snap/src/utils/superstruct.test.ts +++ b/packages/starknet-snap/src/utils/superstruct.test.ts @@ -113,35 +113,11 @@ describe('BaseRequestStruct', () => { assert( { chainId: constants.StarknetChainId.SN_SEPOLIA.toString(), - debugLevel: 'ALL', }, BaseRequestStruct, ), ).not.toThrow(); }); - - it('does not throw error if `debugLevel` is omit', () => { - expect(() => - assert( - { - chainId: constants.StarknetChainId.SN_SEPOLIA.toString(), - }, - BaseRequestStruct, - ), - ).not.toThrow(); - }); - - it('throws error if `debugLevel` is invalid', () => { - expect(() => - assert( - { - chainId: constants.StarknetChainId.SN_SEPOLIA.toString(), - debugLevel: 'Invalid-Debug-Level', - }, - BaseRequestStruct, - ), - ).toThrow(StructError); - }); }); describe('CairoVersionStruct', () => { diff --git a/packages/starknet-snap/src/utils/superstruct.ts b/packages/starknet-snap/src/utils/superstruct.ts index c3f655df..cd3c72f2 100644 --- a/packages/starknet-snap/src/utils/superstruct.ts +++ b/packages/starknet-snap/src/utils/superstruct.ts @@ -29,7 +29,6 @@ import { } from 'superstruct'; import { CAIRO_VERSION_LEGACY, CAIRO_VERSION } from './constants'; -import { LogLevel } from './logger'; export const AddressStruct = refine( string(), @@ -91,8 +90,6 @@ export const AuthorizableStruct = object({ export const BaseRequestStruct = object({ chainId: ChainIdStruct, - // TODO: the debug level should be set by snap rather than pass in from request. - debugLevel: optional(enums(Object.keys(LogLevel))), }); // TODO: refine this to calldata diff --git a/packages/starknet-snap/test/global.ts b/packages/starknet-snap/test/global.ts new file mode 100644 index 00000000..5b9d0755 --- /dev/null +++ b/packages/starknet-snap/test/global.ts @@ -0,0 +1,6 @@ +import * as logger from '../src/utils/logger'; + +// TEMP solution: Switch off logger before each test +exports.mochaGlobalSetup = async function () { + logger.logger.logLevel = logger.LogLevel.OFF; +}; diff --git a/packages/starknet-snap/test/src/index.test.ts b/packages/starknet-snap/test/src/index.test.ts index 327d1c46..fd1fc0b3 100644 --- a/packages/starknet-snap/test/src/index.test.ts +++ b/packages/starknet-snap/test/src/index.test.ts @@ -15,7 +15,6 @@ import { import * as starknetUtils from '../../src/utils/starknetUtils'; import * as createAccountApi from '../../src/createAccount'; import * as keyPairUtils from '../../src/utils/keyPair'; -import * as logger from '../../src/utils/logger'; import { onHomePage, onRpcRequest } from '../../src'; chai.use(sinonChai); @@ -32,8 +31,6 @@ describe('onRpcRequest', function () { afterEach(function () { walletStub.reset(); sandbox.restore(); - // Temp solution: Switch off logger after each test - logger.logger.init('off'); }); it('processes request successfully', async function () { @@ -87,36 +84,6 @@ describe('onRpcRequest', function () { ); } }); - - it('does not hide the error message if an error is thrown and `LogLevel` is not `OFF`', async function () { - mockSnap(); - const createAccountSpy = sandbox.stub(createAccountApi, 'createAccount'); - const keyPairSpy = sandbox.stub(keyPairUtils, 'getAddressKeyDeriver'); - - createAccountSpy.rejects(new Error('Custom Error')); - keyPairSpy.resolvesThis(); - - let expectedError; - - try { - await onRpcRequest({ - origin: 'http://localhost:3000', - request: { - method: 'starkNet_createAccount', - params: { - debugLevel: 'DEBUG', - }, - jsonrpc: '2.0', - id: 1, - }, - }); - } catch (error) { - expectedError = error; - } finally { - expect(expectedError).to.be.instanceOf(SnapError); - expect(expectedError.message).to.equal('Custom Error'); - } - }); }); describe('onHomePage', function () { diff --git a/packages/starknet-snap/test/utils/logger.test.ts b/packages/starknet-snap/test/utils/logger.test.ts deleted file mode 100644 index 68bdb3fe..00000000 --- a/packages/starknet-snap/test/utils/logger.test.ts +++ /dev/null @@ -1,69 +0,0 @@ -import chai, { expect } from 'chai'; -import sinonChai from 'sinon-chai'; -import sinon from 'sinon'; -import * as logutils from '../../src/utils/logger'; - -chai.use(sinonChai); - -describe('Test function: logger', function () { - const logFnSpy = { - log: () => sinon.stub(console, 'log'), - error: () => sinon.stub(console, 'error'), - warn: () => sinon.stub(console, 'warn'), - info: () => sinon.stub(console, 'info'), - trace: () => sinon.stub(console, 'trace'), - debug: () => sinon.stub(console, 'debug'), - }; - const spyempty = () => sinon.stub(logutils, 'emptyLog'); - - afterEach(function () { - sinon.restore(); - sinon.reset(); - }); - - it('when log level set to all, should log every level correctly', function () { - const spy = {}; - for (const key in logFnSpy) { - spy[key] = logFnSpy[key](); - } - - logutils.logger.init('all'); - - for (const key in logFnSpy) { - logutils.logger[key]('log'); - expect(spy[key]).to.have.been.calledOnceWith('log'); - } - }); - - it('when log level set to off, should not log', function () { - const spy = {}; - for (const key in logFnSpy) { - spy[key] = logFnSpy[key](); - } - const _emptySpy = spyempty(); - - logutils.logger.init('off'); - for (const key in logFnSpy) { - logutils.logger[key]('log'); - expect(spy[key]).to.have.been.callCount(0); - } - expect(_emptySpy).to.have.been.callCount(6); - }); - - it('when log level set to info, should log correctly', function () { - const spy = {}; - for (const key in logFnSpy) { - spy[key] = logFnSpy[key](); - } - const _emptySpy = spyempty(); - - logutils.logger.init('info'); - for (const key in logFnSpy) { - logutils.logger[key](`log: ${key}`); - } - expect(spy['info']).to.have.been.calledOnceWith('log: info'); - expect(spy['warn']).to.have.been.calledOnceWith('log: warn'); - expect(spy['error']).to.have.been.calledOnceWith('log: error'); - expect(_emptySpy).to.have.been.callCount(3); - }); -}); diff --git a/packages/wallet-ui/.env.production b/packages/wallet-ui/.env.production index 88e6b2d2..85f8868b 100644 --- a/packages/wallet-ui/.env.production +++ b/packages/wallet-ui/.env.production @@ -1,3 +1,2 @@ REACT_APP_SNAP_ID=npm:@consensys/starknet-snap -REACT_APP_MIN_SNAP_VERSION=2.2.0 -REACT_APP_DEBUG_LEVEL=off \ No newline at end of file +REACT_APP_MIN_SNAP_VERSION=2.2.0 \ No newline at end of file diff --git a/packages/wallet-ui/.env.sample b/packages/wallet-ui/.env.sample index 455db779..8d1011f6 100644 --- a/packages/wallet-ui/.env.sample +++ b/packages/wallet-ui/.env.sample @@ -1,4 +1,3 @@ REACT_APP_SNAP_ID=local:http://localhost:8081 REACT_APP_SNAP_VERSION=* REACT_APP_MIN_SNAP_VERSION=2.2.0 -REACT_APP_DEBUG_LEVEL=all diff --git a/packages/wallet-ui/src/services/useStarkNetSnap.ts b/packages/wallet-ui/src/services/useStarkNetSnap.ts index 9fada059..f44e1262 100644 --- a/packages/wallet-ui/src/services/useStarkNetSnap.ts +++ b/packages/wallet-ui/src/services/useStarkNetSnap.ts @@ -55,17 +55,11 @@ export const useStarkNetSnap = () => { const minSnapVersion = process.env.REACT_APP_MIN_SNAP_VERSION ? process.env.REACT_APP_MIN_SNAP_VERSION : '2.0.1'; - const debugLevel = - process.env.REACT_APP_DEBUG_LEVEL !== undefined - ? process.env.REACT_APP_DEBUG_LEVEL - : 'ALL'; const START_SCAN_INDEX = 0; const MAX_SCANNED = 1; const MAX_MISSED = 1; - const defaultParam = { - debugLevel, - }; + const defaultParam = {}; const connectToSnap = () => { dispatch(enableLoadingWithMessage('Connecting...'));