feat: ignores gasCurrency when passed to the config

.changeset/eight-dragons-behave.md

@@ -0,0 +1,9 @@
+---
+'@celo/celocli': major
+---
+
+The following changes are related to adding support for more fee currencies in the @celo packages.
+
+(BREAKING): global flag `--gasCurrency` changed to accept only whitelisted addresses instead of previously accepting a StableToken or 'auto'
+(BREAKING): `config:set --gasCurrency` is now ignored and not saved to a default config and prints a warning instead
+(ADDED): `celocli network:whitelist` prints the whitelisted feeCurrencies

packages/cli/src/base.ts

@@ -10,7 +10,7 @@ import chalk from 'chalk'
 import net from 'net'
 import Web3 from 'web3'
 import { CustomFlags } from './utils/command'
-import { getGasCurrency, getNodeUrl } from './utils/config'
+import { getNodeUrl } from './utils/config'
 import { requireNodeIsSynced } from './utils/helpers'
 
 export abstract class BaseCommand extends Command {
@@ -43,8 +43,7 @@ export abstract class BaseCommand extends Command {
     }),
     gasCurrency: CustomFlags.gasCurrency({
       description:
-        'Use a specific gas currency for transaction fees (defaults to CELO if no gas currency is supplied)',
-      hidden: true,
+        'Use a specific gas currency for transaction fees (defaults to CELO if no gas currency is supplied). It must be a whitelisted token.',
     }),
     useLedger: Flags.boolean({
       default: false,
@@ -195,10 +194,9 @@ export abstract class BaseCommand extends Command {
       kit.defaultAccount = res.flags.from
     }
 
-    const gasCurrencyFlag = (res.flags.gasCurrency ??
-      (await getGasCurrency(this.config.configDir, kit))) as StrongAddress | 'CELO' | undefined
+    const gasCurrencyFlag = res.flags.gasCurrency as StrongAddress | undefined
 
-    if (gasCurrencyFlag && gasCurrencyFlag !== 'CELO') {
+    if (gasCurrencyFlag) {
       const feeCurrencyWhitelist = await kit.contracts.getFeeCurrencyWhitelist()
       const validFeeCurrencies = await feeCurrencyWhitelist.getWhitelist()
 

packages/cli/src/commands/config/set.test.ts

@@ -0,0 +1,35 @@
+import { ux } from '@oclif/core'
+import { stripAnsiCodesFromNestedArray, testLocally } from '../../test-utils/cliUtils'
+import * as config from '../../utils/config'
+import Set from './set'
+
+process.env.NO_SYNCCHECK = 'true'
+
+afterEach(async () => {
+  jest.clearAllMocks()
+  jest.restoreAllMocks()
+})
+
+describe('config:set cmd', () => {
+  it('shows a warning if gasCurrency is passed', async () => {
+    const consoleMock = jest.spyOn(ux, 'warn')
+    const writeMock = jest.spyOn(config, 'writeConfig')
+
+    await testLocally(Set, ['--gasCurrency', '0x5315e44798395d4a952530d131249fE00f554565'])
+    expect(stripAnsiCodesFromNestedArray(consoleMock.mock.calls as string[][]))
+      .toMatchInlineSnapshot(`
+      [
+        [
+          "
+      Setting a default gasCurrency has been removed from the config, you may still use the --gasCurrency on every command.
+      Did you value this feature a lot and would like to see it back? Let us know at https://github.com/celo-org/developer-tooling/discussions/92",
+        ],
+      ]
+    `)
+    expect(writeMock.mock.calls[0][1]).toMatchInlineSnapshot(`
+      {
+        "node": "http://localhost:8545",
+      }
+    `)
+  })
+})

packages/cli/src/commands/config/set.ts

@@ -1,7 +1,9 @@
+import { ux } from '@oclif/core'
+import chalk from 'chalk'
 import { BaseCommand } from '../../base'
 import { CeloConfig, readConfig, writeConfig } from '../../utils/config'
 export default class Set extends BaseCommand {
-  static description = 'Configure running node information for propogating transactions to network'
+  static description = 'Configure running node information for propagating transactions to network'
 
   static flags = {
     ...BaseCommand.flags,
@@ -12,9 +14,14 @@ export default class Set extends BaseCommand {
   }
 
   static examples = [
+    'set --node mainnet # alias for `forno`',
+    'set --node forno # alias for https://forno.celo.org',
+    'set --node baklava # alias for https://baklava-forno.celo-testnet.org',
+    'set --node alfajores # alias for https://alfajores-forno.celo-testnet.org',
+    'set --node localhost # alias for `local`',
+    'set --node local # alias for http://localhost:8545',
     'set --node ws://localhost:2500',
     'set --node <geth-location>/geth.ipc',
-    'set --gasCurrency 0x874069Fa1Eb16D44d622F2e0Ca25eeA172369bC1',
   ]
 
   requireSynced = false
@@ -23,15 +30,18 @@ export default class Set extends BaseCommand {
     const res = await this.parse(Set)
     const curr = readConfig(this.config.configDir)
     const node = res.flags.node ?? curr.node
-    const gasCurrency = res.flags.gasCurrency ?? curr.gasCurrency
+    const gasCurrency = res.flags.gasCurrency
 
-    await writeConfig(
-      this.config.configDir,
-      {
-        node,
-        gasCurrency,
-      } as CeloConfig,
-      await this.getKit()
-    )
+    if (gasCurrency) {
+      ux.warn(
+        `\nSetting a default gasCurrency has been removed from the config, you may still use the --gasCurrency on every command.\nDid you value this feature a lot and would like to see it back? Let us know at ${chalk.cyan(
+          chalk.bold(`https://github.com/celo-org/developer-tooling/discussions/92`)
+        )}`
+      )
+    }
+
+    await writeConfig(this.config.configDir, {
+      node,
+    } as CeloConfig)
   }
 }

packages/cli/src/transfer-stable-base.ts

@@ -58,11 +58,13 @@ export abstract class TransferStableBase extends BaseCommand {
       failWith(`The ${this._stableCurrency} token was not deployed yet`)
     }
 
-    if (res.flags.gasCurrency) {
-      kit.connection.defaultFeeCurrency = res.flags.gasCurrency
-    }
-    // If gasCurrency is not set, defaults to eip1559 tx
-    const params = res.flags.feeCurrency ? { feeCurrency: kit.connection.defaultFeeCurrency } : {}
+    // NOTE 1: if --gasCurrency is not set, defaults to eip1559 tx
+    // NOTE 2: if --gasCurrency is set by the user, then
+    //     `kit.connection.defaultFeeCurrency` is set in base.ts via
+    //     `kit.setFeeCurrency()`
+    const params = kit.connection.defaultFeeCurrency
+      ? { feeCurrency: kit.connection.defaultFeeCurrency }
+      : {}
 
     const tx = res.flags.comment
       ? stableToken.transferWithComment(to, value.toFixed(), res.flags.comment)
@@ -84,9 +86,9 @@ export abstract class TransferStableBase extends BaseCommand {
           ;[gas, gasPrice, gasBalance, valueBalance] = await Promise.all([
             tx.txo.estimateGas(params),
             kit.connection.gasPrice(kit.connection.defaultFeeCurrency),
-            res.flags.gasCurrency
+            kit.connection.defaultFeeCurrency
               ? // @ts-expect-error abi typing is not 100% correct but works
-                new kit.web3.eth.Contract(ERC20_MOCK_ABI, res.flags.gasCurrency).methods
+                new kit.web3.eth.Contract(ERC20_MOCK_ABI, kit.connection.defaultFeeCurrency).methods
                   .balanceOf(from)
                   .call()
                   .then((x: string) => new BigNumber(x))
@@ -96,7 +98,7 @@ export abstract class TransferStableBase extends BaseCommand {
               .then((token) => token.balanceOf(from)),
           ])
           const gasValue = new BigNumber(gas).times(gasPrice as string)
-          if (res.flags.gasCurrency) {
+          if (kit.connection.defaultFeeCurrency) {
             return gasBalance.gte(gasValue) && valueBalance.gte(value)
           }
           return valueBalance.gte(value.plus(gasValue))

packages/cli/src/utils/command.ts

@@ -48,10 +48,7 @@ const parseAddress: ParseFn<StrongAddress> = async (input) => {
     throw new CLIError(`${input} is not a valid address`)
   }
 }
-const parseGasCurrency: ParseFn<StrongAddress | 'CELO'> = async (input) => {
-  if (input.toUpperCase() === 'CELO') {
-    return 'CELO'
-  }
+const parseGasCurrency: ParseFn<StrongAddress> = async (input) => {
   if (Web3.utils.isAddress(input)) {
     return input as StrongAddress
   } else {
@@ -166,10 +163,10 @@ export const CustomFlags = {
     description: 'Account Address',
     helpValue: '0xc1912fEE45d61C87Cc5EA59DaE31190FFFFf232d',
   }),
-  gasCurrency: Flags.custom<StrongAddress | 'CELO'>({
+  gasCurrency: Flags.custom<StrongAddress>({
     parse: parseGasCurrency,
-    description: 'A whitelisted feeCurrency or CELO to use native eip1559 transactions',
-    helpValue: '0',
+    description: 'A whitelisted feeCurrency',
+    helpValue: '0x1234567890123456789012345678901234567890',
   }),
   ecdsaPublicKey: Flags.custom({
     parse: parseEcdsaPublicKey,

packages/cli/src/utils/config.test.ts

@@ -1,129 +1,75 @@
-import { StableToken, StrongAddress } from '@celo/base'
-import { newKitFromWeb3 } from '@celo/contractkit'
-import { testWithGanache } from '@celo/dev-utils/lib/ganache-test'
 import * as fs from 'fs-extra'
 import { tmpdir } from 'os'
 import { join } from 'path'
-import { CeloConfig, LegacyCeloConfig, getGasCurrency, writeConfig } from './config'
+import { CeloConfig, readConfig, writeConfig } from './config'
 
 // NOTE: for some reason if I don't mock the whole module, jest fails
 // to spy on outputJSONSync
 jest.mock('fs-extra', () => ({ __esModule: true, ...jest.requireActual('fs-extra') }))
 
-const PATH = tmpdir()
+const getPaths = () => {
+  const dir = tmpdir()
+  return [dir, join(dir, 'config.json')]
+}
+
 const spy = jest.spyOn(fs, 'outputJSONSync')
 
 beforeEach(() => {
   spy.mockClear()
 })
 
-testWithGanache('config', (web3) => {
-  const kit = newKitFromWeb3(web3)
-  describe('writeConfig', () => {
-    test('empty config', async () => {
-      writeConfig(PATH, {} as CeloConfig, kit)
+describe('writeConfig', () => {
+  it('accepts an empty config', async () => {
+    const [dir, file] = getPaths()
+    writeConfig(dir, {} as CeloConfig)
 
-      expect(spy).toHaveBeenCalledTimes(1)
-      expect(spy.mock.calls[0]).toHaveLength(2)
-      expect(spy.mock.calls[0][0]).toEqual(join(PATH, 'config.json'))
-      expect(spy.mock.calls[0][1]).toMatchInlineSnapshot(`
+    expect(spy).toHaveBeenCalledTimes(1)
+    expect(spy.mock.calls[0]).toHaveLength(2)
+    expect(spy.mock.calls[0][0]).toEqual(file)
+    expect(spy.mock.calls[0][1]).toMatchInlineSnapshot(`
         {
           "node": "http://localhost:8545",
         }
       `)
-    })
-    test('node', async () => {
-      await writeConfig(PATH, { node: 'SOME_URL' }, kit)
-      expect(spy.mock.calls[0][1]).toMatchInlineSnapshot(`
+  })
+  it('accepts node', async () => {
+    const [dir] = getPaths()
+    await writeConfig(dir, { node: 'SOME_URL' })
+    expect(spy.mock.calls[0][1]).toMatchInlineSnapshot(`
         {
           "node": "SOME_URL",
         }
       `)
-    })
-    test('gasCurrency auto (legacy)', async () => {
-      await writeConfig(PATH, { gasCurrency: 'auto' } as LegacyCeloConfig, kit)
-      expect(spy.mock.calls[0][1]).toMatchInlineSnapshot(`
-        {
-          "node": "http://localhost:8545",
-        }
-      `)
-    })
-    test('gasCurrency StableToken (legacy)', async () => {
-      await writeConfig(PATH, { gasCurrency: 'cUSD' } as LegacyCeloConfig, kit)
-      expect(spy.mock.calls[0][1]).toMatchInlineSnapshot(`
-        {
-          "gasCurrency": "0x5315e44798395d4a952530d131249fE00f554565",
-          "node": "http://localhost:8545",
-        }
-      `)
-    })
-    test('gasCurrency CELO', async () => {
-      await writeConfig(PATH, { gasCurrency: 'CELO' } as LegacyCeloConfig, kit)
-      expect(spy.mock.calls[0][1]).toMatchInlineSnapshot(`
-        {
-          "node": "http://localhost:8545",
-        }
-      `)
-    })
-    test('gasCurrency badly formatted address', async () => {
-      await expect(
-        writeConfig(PATH, { gasCurrency: '0x123' as StrongAddress } as CeloConfig, kit)
-      ).rejects.toMatchInlineSnapshot(`[Error: Invalid address 0x123]`)
-    })
-    test('gasCurrency wrong address', async () => {
-      await expect(
-        writeConfig(
-          PATH,
-          {
-            gasCurrency: '0x0000000000000000000000000000000000000000' as StrongAddress,
-          } as CeloConfig,
-          kit
-        )
-      ).rejects.toMatchInlineSnapshot(`
-        [Error: 0x0000000000000000000000000000000000000000 is not a valid fee currency. Available currencies:
-        0x5315e44798395d4a952530d131249fE00f554565 - Celo Dollar (cUSD)
-        0x965D352283a3C8A016b9BBbC9bf6306665d495E7 - Celo Brazilian Real (cREAL)
-        0xdD66C23e07b4D6925b6089b5Fe6fc9E62941aFE8 - Celo Euro (cEUR)]
-      `)
-    })
-
-    test('gasCurrency address', async () => {
-      await writeConfig(
-        PATH,
-        {
-          gasCurrency: (await kit.contracts.getStableToken(StableToken.cEUR)).address,
-        } as CeloConfig,
-        kit
-      )
-      expect(spy.mock.calls[0][1]).toMatchInlineSnapshot(`
-        {
-          "gasCurrency": "0xdD66C23e07b4D6925b6089b5Fe6fc9E62941aFE8",
-          "node": "http://localhost:8545",
-        }
-      `)
-    })
   })
+})
 
-  describe('getGasCurrency', () => {
-    test('empty', async () => {
-      fs.outputJSONSync(join(PATH, 'config.json'), {})
-      expect(await getGasCurrency(PATH, kit)).toBe(undefined)
-    })
-    test('CELO', async () => {
-      fs.outputJSONSync(join(PATH, 'config.json'), { getGasCurrency: 'CELO' })
-      expect(await getGasCurrency(PATH, kit)).toBe(undefined)
-    })
-    test('address', async () => {
-      fs.outputJSONSync(join(PATH, 'config.json'), {
-        gasCurrency: '0x0000000000000000000000000000000000000000',
-      })
-      expect(await getGasCurrency(PATH, kit)).toBe('0x0000000000000000000000000000000000000000')
-    })
-    test('legacy', async () => {
-      fs.outputJSONSync(join(PATH, 'config.json'), { gasCurrency: 'cEUR' })
-      expect(await getGasCurrency(PATH, kit)).toBe(
-        (await kit.contracts.getStableToken(StableToken.cEUR)).address
-      )
-    })
+describe('readConfig', () => {
+  it('reads arbitrary JSON at the file location', () => {
+    const [dir, file] = getPaths()
+    fs.writeJsonSync(file, { foo: 'bar' })
+    expect(readConfig(dir)).toMatchInlineSnapshot(`
+      {
+        "foo": "bar",
+        "node": "http://localhost:8545",
+      }
+    `)
+  })
+  it('translates legacy values into their new aliases', () => {
+    const [dir, file] = getPaths()
+    fs.writeJsonSync(file, { nodeUrl: 'bar' })
+    expect(readConfig(dir)).toMatchInlineSnapshot(`
+      {
+        "node": "bar",
+      }
+    `)
+  })
+  it('translates legacy values into their new aliases', () => {
+    const [dir, file] = getPaths()
+    fs.writeJsonSync(file, { gasCurrency: 'CELO' })
+    expect(readConfig(dir)).toMatchInlineSnapshot(`
+      {
+        "node": "http://localhost:8545",
+      }
+    `)
   })
 })