Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BCF-2492 Remove evm default chain id #10490

Merged
merged 56 commits into from
Sep 14, 2023
+496 −483
Merged

BCF-2492 Remove evm default chain id #10490

Show file tree
Hide file tree
Changes from 51 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
2de282d
Remove evm default chain id usage
ilija42 Sep 5, 2023
63d8a6f
Remove the rest of evm default chain id usage
ilija42 Sep 5, 2023
99096ab
Enforce evmChainID to be defined in all jobspecs where present
ilija42 Sep 5, 2023
48ca938
Remove setdefault chain id from NewLegacyChainsWithChain() mock
ilija42 Sep 5, 2023
6e1d5a0
Change cltest NewApplicationWithConfig to init evmopts if evm enabled
ilija42 Sep 6, 2023
6fbea6b
Fix pipeline & eth_keys & jobs controller tests to encode evmChainID
ilija42 Sep 6, 2023
58df0cf
Add evmChainID to test tomls and to VRFSpecParams testspec struct
ilija42 Sep 6, 2023
587c0d8
Add evmChainID to vrf submit_tx pipeline vars and testspec source
ilija42 Sep 6, 2023
fbcd0b6
Add evmChainID to testspecs job specs where missing
ilija42 Sep 6, 2023
ecca443
Add evmChainID flags to shell local tests
ilija42 Sep 6, 2023
6d04fc6
Change TestORM_CreateJob_OCR_DuplicatedContractAddress
ilija42 Sep 6, 2023
f8c55ea
Change orm validation missing evmChainID error message
ilija42 Sep 6, 2023
37790d8
Change tests that relied on default chainID to use replacement func
ilija42 Sep 6, 2023
1b79c95
Add deprecated tag to default chainID helper funcs and update err msg
ilija42 Sep 6, 2023
5881147
comment out test case in TestShell_ReplayBlock
ilija42 Sep 6, 2023
082174a
Merge remote-tracking branch 'origin/develop' into BCF-2492-remove-de…
ilija42 Sep 6, 2023
b4236d3
minor tests fix
ilija42 Sep 6, 2023
b887874
Add evmchainID to spec to keepers and flux integration tests
ilija42 Sep 6, 2023
8868ada
Add evmChainID to vrfv2 pipeline vars
ilija42 Sep 7, 2023
fff8fc6
Fix evmChainID parsing in estimategas pipeline task
ilija42 Sep 7, 2023
598b341
Add evmChainID to estimate gas and eth call pipeline tasks vars spec
ilija42 Sep 7, 2023
4e3f8b4
Fix/Change cmds tests to use evmChainID
ilija42 Sep 7, 2023
096706b
Fix vrf integration tests and helpers to use evmChainID
ilija42 Sep 7, 2023
e133425
Add evmChainID parsing string as default val to pipeline tasks that u…
ilija42 Sep 8, 2023
93b392c
Add evmChainID to specs in internal features_test
ilija42 Sep 8, 2023
0c9c3e3
Add evmChainID to vrf and ocr smoke test job specs
ilija42 Sep 8, 2023
89d112e
Fix evmChainID in keepers tests
ilija42 Sep 8, 2023
29f84d7
Fix evmChainID in fluxmonitorv2 tests
ilija42 Sep 8, 2023
353557f
Merge remote-tracking branch 'origin/develop' into BCF-2492-remove-de…
ilija42 Sep 11, 2023
c728bda
Fix estimategas, ethcall/tx default evmChainID values for parsing vars
ilija42 Sep 11, 2023
dfc4c5c
Remove default tag from EstimateGasLimitTask.EVMChainID
ilija42 Sep 11, 2023
2b38531
Merge remote-tracking branch 'origin/develop' into BCF-2492-remove-de…
ilija42 Sep 11, 2023
0ade421
Add evmChainID to pipeline vars in directRequest handleOracleRequest
ilija42 Sep 11, 2023
f93ec01
Add evmChainID to all pipeline vars where needed
ilija42 Sep 11, 2023
87bf52d
Merge remote-tracking branch 'origin/develop' into BCF-2492-remove-de…
ilija42 Sep 11, 2023
b8fb5ec
Merge remote-tracking branch 'origin/develop' into BCF-2492-remove-de…
ilija42 Sep 11, 2023
fe42327
Merge remote-tracking branch 'origin/develop' into BCF-2492-remove-de…
ilija42 Sep 12, 2023
2a6624b
Resolve develop merge issues
ilija42 Sep 12, 2023
568632b
Fix evmChainID pipeline vars parsing
ilija42 Sep 12, 2023
e8ca0f5
Add mock calls to direct request tests for EVMChainID() call
ilija42 Sep 12, 2023
3a2b560
Run make go generate
ilija42 Sep 12, 2023
792ffac
Add unit tests for job creation evmChainID validation
ilija42 Sep 12, 2023
d3d05da
Fix keepers and flux smoke tests to have evmChainID in the jobspec
ilija42 Sep 12, 2023
9f8e1b7
Fix smoke test job spec evmChainID for ocr, reorg, runlog and vrf
ilija42 Sep 12, 2023
91b1e29
Fix integration tests by adding chainID to OCR task and Bootstrap Spec
ilija42 Sep 12, 2023
3a18f6a
Fix integrations test OCRTaskJobSpec struct field naming case
ilija42 Sep 12, 2023
490d153
minor fix to keeper job creation smoke test
ilija42 Sep 13, 2023
51e66de
Merge remote-tracking branch 'origin/develop' into BCF-2492-remove-de…
ilija42 Sep 13, 2023
aafc41c
Merge remote-tracking branch 'origin/develop' into BCF-2492-remove-de…
ilija42 Sep 13, 2023
ef7d1a3
Remove unnecessary error wrap in ValidatedOracleSpecToml
ilija42 Sep 13, 2023
220050f
Remove shell_remote test case that relied on default evmChainID
ilija42 Sep 13, 2023
4982867
Remove keystore migrate as all v1 keys should have been migrated by now
ilija42 Sep 14, 2023
24556cb
Use %q in (c *LegacyChains) Get() for better error msg output
ilija42 Sep 14, 2023
575b15b
Merge remote-tracking branch 'origin/develop' into BCF-2492-remove-de…
ilija42 Sep 14, 2023
164bb39
Fix TestShell_ReplayBlocks test by fixing err message assert
ilija42 Sep 14, 2023
ba4ac9e
Add changes to CHANGELOG.md
ilija42 Sep 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 2 additions & 17 deletions core/chains/evm/chain.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ type LegacyChains struct {
//
//go:generate mockery --quiet --name LegacyChainContainer --output ./mocks/ --case=underscore
type LegacyChainContainer interface {
SetDefault(Chain)
Default() (Chain, error)
Get(id string) (Chain, error)
Len() int
List(ids ...string) ([]Chain, error)
Expand All @@ -100,27 +98,14 @@ func (c *LegacyChains) ChainNodeConfigs() evmtypes.Configs {
return c.cfgs
}

// TODO BCR-2510 this may not be needed if EVM is not enabled by default
func (c *LegacyChains) SetDefault(dflt Chain) {
c.dflt = dflt
}

func (c *LegacyChains) Default() (Chain, error) {
if c.dflt == nil {
return nil, fmt.Errorf("no default chain specified")
}
return c.dflt, nil
}

// backward compatibility.
// eth keys are represented as multiple types in the code base;
// *big.Int, string, and int64. this lead to special 'default' handling
// of nil big.Int and empty string.
// *big.Int, string, and int64.
//
// TODO BCF-2507 unify the type system
func (c *LegacyChains) Get(id string) (Chain, error) {
if id == nilBigInt.String() || id == emptyString {
return c.Default()
return nil, fmt.Errorf("invalid chain id requested: %s", id)
ilija42 marked this conversation as resolved.
Show resolved Hide resolved
}
return c.ChainsKV.Get(id)
}
Expand Down
18 changes: 0 additions & 18 deletions core/chains/evm/config/mocks/chain_scoped_config.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 9 additions & 9 deletions core/chains/evm/headtracker/head_tracker_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ func TestHeadTracker_SwitchesToLongestChainWithHeadSamplingEnabled(t *testing.T)
ethClient := evmtest.NewEthClientMockWithDefaultChain(t)

checker := commonmocks.NewHeadTrackable[*evmtypes.Head, gethCommon.Hash](t)
orm := headtracker.NewORM(db, logger, config.Database(), *config.DefaultChainID())
orm := headtracker.NewORM(db, logger, config.Database(), *evmtest.MustGetDefaultChainID(t, config.EVMConfigs()))
csCfg := evmtest.NewChainScopedConfig(t, config)
ht := createHeadTrackerWithChecker(t, ethClient, csCfg.EVM(), csCfg.EVM().HeadTracker(), orm, checker)

Expand Down Expand Up @@ -779,7 +779,7 @@ func TestHeadTracker_Backfill(t *testing.T) {
}

ethClient := evmtest.NewEthClientMock(t)
ethClient.On("ConfiguredChainID", mock.Anything).Return(cfg.DefaultChainID(), nil)
ethClient.On("ConfiguredChainID", mock.Anything).Return(evmtest.MustGetDefaultChainID(t, cfg.EVMConfigs()), nil)
ht := createHeadTrackerWithNeverSleeper(t, ethClient, cfg, orm)

err := ht.Backfill(ctx, &h12, 2)
Expand All @@ -796,7 +796,7 @@ func TestHeadTracker_Backfill(t *testing.T) {
}

ethClient := evmtest.NewEthClientMock(t)
ethClient.On("ConfiguredChainID", mock.Anything).Return(cfg.DefaultChainID(), nil)
ethClient.On("ConfiguredChainID", mock.Anything).Return(evmtest.MustGetDefaultChainID(t, cfg.EVMConfigs()), nil)
ethClient.On("HeadByHash", mock.Anything, head10.Hash).
Return(&head10, nil)

Expand Down Expand Up @@ -832,7 +832,7 @@ func TestHeadTracker_Backfill(t *testing.T) {
}

ethClient := evmtest.NewEthClientMock(t)
ethClient.On("ConfiguredChainID", mock.Anything).Return(cfg.DefaultChainID(), nil)
ethClient.On("ConfiguredChainID", mock.Anything).Return(evmtest.MustGetDefaultChainID(t, cfg.EVMConfigs()), nil)

ht := createHeadTrackerWithNeverSleeper(t, ethClient, cfg, orm)

Expand Down Expand Up @@ -865,7 +865,7 @@ func TestHeadTracker_Backfill(t *testing.T) {
}

ethClient := evmtest.NewEthClientMock(t)
ethClient.On("ConfiguredChainID", mock.Anything).Return(cfg.DefaultChainID(), nil)
ethClient.On("ConfiguredChainID", mock.Anything).Return(evmtest.MustGetDefaultChainID(t, cfg.EVMConfigs()), nil)

ht := createHeadTrackerWithNeverSleeper(t, ethClient, cfg, orm)

Expand All @@ -883,7 +883,7 @@ func TestHeadTracker_Backfill(t *testing.T) {
orm := headtracker.NewORM(db, logger, cfg.Database(), cltest.FixtureChainID)

ethClient := evmtest.NewEthClientMock(t)
ethClient.On("ConfiguredChainID", mock.Anything).Return(cfg.DefaultChainID(), nil)
ethClient.On("ConfiguredChainID", mock.Anything).Return(evmtest.MustGetDefaultChainID(t, cfg.EVMConfigs()), nil)
ethClient.On("HeadByHash", mock.Anything, head0.Hash).
Return(&head0, nil)

Expand Down Expand Up @@ -911,7 +911,7 @@ func TestHeadTracker_Backfill(t *testing.T) {
}

ethClient := evmtest.NewEthClientMock(t)
ethClient.On("ConfiguredChainID", mock.Anything).Return(cfg.DefaultChainID(), nil)
ethClient.On("ConfiguredChainID", mock.Anything).Return(evmtest.MustGetDefaultChainID(t, cfg.EVMConfigs()), nil)
ethClient.On("HeadByHash", mock.Anything, head10.Hash).
Return(&head10, nil).
Once()
Expand Down Expand Up @@ -942,7 +942,7 @@ func TestHeadTracker_Backfill(t *testing.T) {
}

ethClient := evmtest.NewEthClientMock(t)
ethClient.On("ConfiguredChainID", mock.Anything).Return(cfg.DefaultChainID(), nil)
ethClient.On("ConfiguredChainID", mock.Anything).Return(evmtest.MustGetDefaultChainID(t, cfg.EVMConfigs()), nil)
ethClient.On("HeadByHash", mock.Anything, head10.Hash).
Return(&head10, nil)
ethClient.On("HeadByHash", mock.Anything, head8.Hash).
Expand All @@ -967,7 +967,7 @@ func TestHeadTracker_Backfill(t *testing.T) {
logger := logger.TestLogger(t)
orm := headtracker.NewORM(db, logger, cfg.Database(), cltest.FixtureChainID)
ethClient := evmtest.NewEthClientMock(t)
ethClient.On("ConfiguredChainID", mock.Anything).Return(cfg.DefaultChainID(), nil)
ethClient.On("ConfiguredChainID", mock.Anything).Return(evmtest.MustGetDefaultChainID(t, cfg.EVMConfigs()), nil)
ethClient.On("HeadByHash", mock.Anything, h14.Hash).Return(&h14, nil).Once()
ethClient.On("HeadByHash", mock.Anything, h13.Hash).Return(&h13, nil).Once()
ethClient.On("HeadByHash", mock.Anything, h12.Hash).Return(nil, errors.New("not found")).Once()
Expand Down
31 changes: 0 additions & 31 deletions core/chains/evm/mocks/legacy_chain_container.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion core/chains/evm/txmgr/evm_tx_store_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ func TestORM_SetBroadcastBeforeBlockNum(t *testing.T) {
t.Run("only updates evm.tx_attempts for the current chain", func(t *testing.T) {
require.NoError(t, ethKeyStore.Add(fromAddress, testutils.SimulatedChainID))
require.NoError(t, ethKeyStore.Enable(fromAddress, testutils.SimulatedChainID))
etxThisChain := cltest.MustInsertUnconfirmedEthTxWithBroadcastLegacyAttempt(t, txStore, 1, fromAddress, cfg.DefaultChainID())
etxThisChain := cltest.MustInsertUnconfirmedEthTxWithBroadcastLegacyAttempt(t, txStore, 1, fromAddress, cfg.EVM().ChainID())
etxOtherChain := cltest.MustInsertUnconfirmedEthTxWithBroadcastLegacyAttempt(t, txStore, 0, fromAddress, testutils.SimulatedChainID)

require.NoError(t, txStore.SetBroadcastBeforeBlockNum(headNum, chainID))
Expand Down
31 changes: 15 additions & 16 deletions core/cmd/eth_keys_commands_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/smartcontractkit/chainlink/v2/core/assets"
"github.com/smartcontractkit/chainlink/v2/core/cmd"
"github.com/smartcontractkit/chainlink/v2/core/internal/cltest"
"github.com/smartcontractkit/chainlink/v2/core/internal/testutils"
"github.com/smartcontractkit/chainlink/v2/core/services/chainlink"
"github.com/smartcontractkit/chainlink/v2/core/utils"
"github.com/smartcontractkit/chainlink/v2/core/web/presenters"
Expand Down Expand Up @@ -182,28 +183,21 @@ func TestShell_CreateETHKey(t *testing.T) {
require.NoError(t, err)
require.Equal(t, 1, len(keys))

// create a key on the default chain
set := flag.NewFlagSet("test", 0)
cltest.FlagSetApplyFromAction(client.CreateETHKey, set, "")
c := cli.NewContext(nil, set, nil)
assert.NoError(t, client.CreateETHKey(c))

// create the key on a specific chainID
id := big.NewInt(0)

set = flag.NewFlagSet("test", 0)
set := flag.NewFlagSet("test", 0)
cltest.FlagSetApplyFromAction(client.CreateETHKey, set, "")

require.NoError(t, set.Set("evmChainID", ""))
require.NoError(t, set.Set("evm-chain-id", testutils.FixtureChainID.String()))

c = cli.NewContext(nil, set, nil)
require.NoError(t, set.Parse([]string{"-evmChainID", id.String()}))
c := cli.NewContext(nil, set, nil)
require.NoError(t, set.Parse([]string{"-evm-chain-id", id.String()}))
assert.NoError(t, client.CreateETHKey(c))

cltest.AssertCount(t, db, "evm.key_states", 3)
cltest.AssertCount(t, db, "evm.key_states", 2)
keys, err = app.KeyStore.Eth().GetAll()
require.NoError(t, err)
require.Equal(t, 3, len(keys))
require.Equal(t, 2, len(keys))
}

func TestShell_DeleteETHKey(t *testing.T) {
Expand Down Expand Up @@ -308,9 +302,12 @@ func TestShell_ImportExportETHKey_NoChains(t *testing.T) {

// Import the key
set = flag.NewFlagSet("test", 0)
set.String("old-password", "../internal/fixtures/incorrect_password.txt", "")
err = set.Parse([]string{keyfilepath})
require.NoError(t, err)
cltest.FlagSetApplyFromAction(client.ImportETHKey, set, "")

require.NoError(t, set.Set("evmChainID", testutils.FixtureChainID.String()))
require.NoError(t, set.Set("old-password", "../internal/fixtures/incorrect_password.txt"))
require.NoError(t, set.Parse([]string{keyfilepath}))

c = cli.NewContext(nil, set, nil)
err = client.ImportETHKey(c)
require.NoError(t, err)
Expand Down Expand Up @@ -412,6 +409,8 @@ func TestShell_ImportExportETHKey_WithChains(t *testing.T) {
set = flag.NewFlagSet("test", 0)
cltest.FlagSetApplyFromAction(client.ImportETHKey, set, "")

require.NoError(t, set.Set("evmChainID", testutils.FixtureChainID.String()))
require.NoError(t, set.Set("evmChainID", testutils.FixtureChainID.String()))
require.NoError(t, set.Set("old-password", "../internal/fixtures/incorrect_password.txt"))
require.NoError(t, set.Parse([]string{keyfilepath}))

Expand Down
4 changes: 4 additions & 0 deletions core/cmd/evm_transaction_commands_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import (
evmtypes "github.com/smartcontractkit/chainlink/v2/core/chains/evm/types"
"github.com/smartcontractkit/chainlink/v2/core/cmd"
"github.com/smartcontractkit/chainlink/v2/core/internal/cltest"
"github.com/smartcontractkit/chainlink/v2/core/internal/testutils"
"github.com/smartcontractkit/chainlink/v2/core/internal/testutils/evmtest"
"github.com/smartcontractkit/chainlink/v2/core/services/chainlink"
"github.com/smartcontractkit/chainlink/v2/core/store/models"
)
Expand Down Expand Up @@ -160,6 +162,7 @@ func TestShell_SendEther_From_Txm(t *testing.T) {
amount := "100.5"
to := "0x342156c8d3bA54Abc67920d35ba1d1e67201aC9C"
require.NoError(t, set.Parse([]string{amount, fromAddress.Hex(), to}))
require.NoError(t, set.Set("id", evmtest.MustGetDefaultChainID(t, app.Config.EVMConfigs()).String()))

cliapp := cli.NewApp()
c := cli.NewContext(cliapp, set, nil)
Expand Down Expand Up @@ -215,6 +218,7 @@ func TestShell_SendEther_From_Txm_WEI(t *testing.T) {
set := flag.NewFlagSet("sendether", 0)
cltest.FlagSetApplyFromAction(client.SendEther, set, "")

require.NoError(t, set.Set("id", testutils.FixtureChainID.String()))
require.NoError(t, set.Set("wei", "false"))

amount := "1000000000000000000"
Expand Down
4 changes: 2 additions & 2 deletions core/cmd/forwarders_commands_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func TestShell_TrackEVMForwarder(t *testing.T) {
cltest.FlagSetApplyFromAction(client.TrackForwarder, set, "")

require.NoError(t, set.Set("address", "0x5431F5F973781809D18643b87B44921b11355d81"))
require.NoError(t, set.Set("evmChainID", id.String()))
require.NoError(t, set.Set("evm-chain-id", id.String()))

err := client.TrackForwarder(cli.NewContext(nil, set, nil))
require.NoError(t, err)
Expand Down Expand Up @@ -122,7 +122,7 @@ func TestShell_TrackEVMForwarder_BadAddress(t *testing.T) {
cltest.FlagSetApplyFromAction(client.TrackForwarder, set, "")

require.NoError(t, set.Set("address", "0xWrongFormatAddress"))
require.NoError(t, set.Set("evmChainID", id.String()))
require.NoError(t, set.Set("evm-chain-id", id.String()))

err := client.TrackForwarder(cli.NewContext(nil, set, nil))
require.Contains(t, err.Error(), "could not decode address: invalid hex string")
Expand Down
15 changes: 3 additions & 12 deletions core/cmd/shell_local.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,18 +375,6 @@ func (s *Shell) runNode(c *cli.Context) error {

legacyEVMChains := app.GetRelayers().LegacyEVMChains()

// By passing in a function we can be lazy trying to look up a default
// chain - if there are no existing keys, there is no need to check for
// a chain ID
DefaultEVMChainIDFunc := func() (*big.Int, error) {
def, err2 := legacyEVMChains.Default()
if err2 != nil {
return nil, errors.Wrap(err2, "cannot get default EVM chain ID; no default EVM chain available")
}
return def.ID(), nil
}
err = keyStore.Migrate(s.Config.Password().VRF(), DefaultEVMChainIDFunc)
ilija42 marked this conversation as resolved.
Show resolved Hide resolved

if s.Config.EVMEnabled() {
if err != nil {
return errors.Wrap(err, "error migrating keystore")
Expand All @@ -396,6 +384,9 @@ func (s *Shell) runNode(c *cli.Context) error {
return fmt.Errorf("error listing legacy evm chains: %w", err)
}
for _, ch := range chainList {
if err = keyStore.Migrate(s.Config.Password().VRF(), ch.ID()); err != nil {
return errors.Wrap(err, "error migrating keystore")
}
if ch.Config().EVM().AutoCreateKey() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me, but I'm not 100% sure

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you find the author of this code and confirm the removal? i'm surprised that we have to touch keystore migrations with this change, and we have to 100% certain that this is ok

Copy link
Contributor Author

@ilija42 ilija42 Sep 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like @samsondav and @RyanRHall touched this code the most. Sam already approved, maybe Ryan can give some input here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

am i reading it correctly - the existing behavior was to migrate to the default chain. now we are going to migrate to all of the chains?

my reading of Migrate is that it does a bunch of stuff to a bunch of different kind of keys. i'm not convinced it was intended to be called multiple times.

maybe we need to split it up and call the evm-specific part once per chain? or change it to take a slice of evm keys

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@krehermann removing the keystore migrations is 100% fine. That code is over 2 years stale at some point. No need to maintain it. Asking @ilija42 to create a ticket to fully deprecate the v1 keystore at some point also :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a brief discussion with @RyanRHall it seems like this code is obsolete, so it can be just deleted.
We don't permit creating new v1 keys anymore. So all v1 keys should have been migrated by now.

lggr.Debugf("AutoCreateKey=true, will ensure EVM key for chain %s", ch.ID())
err2 := app.GetKeyStore().Eth().EnsureKeys(ch.ID())
Expand Down
4 changes: 3 additions & 1 deletion core/cmd/shell_local_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ func TestShell_RebroadcastTransactions_Txm(t *testing.T) {
set := flag.NewFlagSet("test", 0)
cltest.FlagSetApplyFromAction(client.RebroadcastTransactions, set, "")

require.NoError(t, set.Set("evmChainID", testutils.FixtureChainID.String()))
require.NoError(t, set.Set("beginningNonce", strconv.FormatUint(beginningNonce, 10)))
require.NoError(t, set.Set("endingNonce", strconv.FormatUint(endingNonce, 10)))
require.NoError(t, set.Set("gasPriceWei", "100000000000"))
Expand Down Expand Up @@ -402,6 +403,7 @@ func TestShell_RebroadcastTransactions_OutsideRange_Txm(t *testing.T) {
set := flag.NewFlagSet("test", 0)
cltest.FlagSetApplyFromAction(client.RebroadcastTransactions, set, "")

require.NoError(t, set.Set("evmChainID", testutils.FixtureChainID.String()))
require.NoError(t, set.Set("beginningNonce", strconv.FormatUint(uint64(beginningNonce), 10)))
require.NoError(t, set.Set("endingNonce", strconv.FormatUint(uint64(endingNonce), 10)))
require.NoError(t, set.Set("gasPriceWei", gasPrice.String()))
Expand Down Expand Up @@ -480,9 +482,9 @@ func TestShell_RebroadcastTransactions_AddressCheck(t *testing.T) {
}

set := flag.NewFlagSet("test", 0)
set.Set("evmChainID", testutils.SimulatedChainID.String())
cltest.FlagSetApplyFromAction(client.RebroadcastTransactions, set, "")

require.NoError(t, set.Set("evmChainID", testutils.FixtureChainID.String()))
require.NoError(t, set.Set("address", fromAddress.Hex()))
require.NoError(t, set.Set("password", "../internal/fixtures/correct_password.txt"))
c := cli.NewContext(nil, set, nil)
Expand Down
6 changes: 1 addition & 5 deletions core/cmd/shell_remote_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,8 @@ func TestShell_ReplayBlocks(t *testing.T) {
cltest.FlagSetApplyFromAction(client.ReplayFromBlock, set, "")

require.NoError(t, set.Set("block-number", "42"))

c := cli.NewContext(nil, set, nil)
assert.NoError(t, client.ReplayFromBlock(c))

require.NoError(t, set.Set("evm-chain-id", "12345678"))
c = cli.NewContext(nil, set, nil)
c := cli.NewContext(nil, set, nil)
assert.ErrorContains(t, client.ReplayFromBlock(c), "evmChainID does not match any local chains")

require.NoError(t, set.Set("evm-chain-id", "0"))
Expand Down
Loading
Loading