Repository: onflow/flow-cli (go)

Issue:
Remove contract bug
### Problem
If you try to remove a contract from the account in a folder where flow.json doesn't exist you get a crash.

```
flow accounts remove-contract FungibleToken --signer test -n testnet                                                                                                                       
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x10 pc=0x1027bcf40]

goroutine 1 [running]:
github.com/onflow/flow-cli/pkg/flowkit.(*Account).Address(...)
	github.com/onflow/flow-cli/pkg/flowkit/account.go:40
github.com/onflow/flow-cli/pkg/flowkit.NewRemoveAccountContractTransaction(0x0, 0x16dd979ee, 0xd, 0x1400047ede8, 0x1400066fab8, 0x102a8a5c0)
	github.com/onflow/flow-cli/pkg/flowkit/transaction.go:89 +0x20
github.com/onflow/flow-cli/pkg/flowkit/services.(*Accounts).RemoveContract(0x1400023b8c0, 0x0, 0x16dd979ee, 0xd, 0x0, 0x0, 0x0)
	github.com/onflow/flow-cli/pkg/flowkit/services/accounts.go:309 +0x54
github.com/onflow/flow-cli/internal/accounts.removeContract(0x1400009b180, 0x1, 0x5, 0x1031ea528, 0x1400023de20, 0x0, 0x0, 0x102bec043, 0x4, 0x0, ...)
	github.com/onflow/flow-cli/internal/accounts/contract-remove.go:58 +0xac
github.com/onflow/flow-cli/internal/command.Command.AddToParent.func1(0x103a09ce0, 0x1400009b180, 0x1, 0x5)
	github.com/onflow/flow-cli/internal/command/command.go:116 +0x2fc
github.com/spf13/cobra.(*Command).execute(0x103a09ce0, 0x1400003c0b0, 0x5, 0x5, 0x103a09ce0, 0x1400003c0b0)
	github.com/spf13/cobra@v1.1.3/command.go:856 +0x1d0
github.com/spf13/cobra.(*Command).ExecuteC(0x14000123180, 0x140004bff10, 0x1, 0x1)
	github.com/spf13/cobra@v1.1.3/command.go:960 +0x258
github.com/spf13/cobra.(*Command).Execute(...)
	github.com/spf13/cobra@v1.1.3/command.go:897
main.main()

```


Pull request:
added errors to ByName and ByNetwork for accounts
Closes #333 

Added error to ByName and ByNetwork methods in Accounts. Explicitly check everywhere and return proper errors.


---
For contributor use:

- [x] Targeted PR against `master` branch
- [x] Linked to Github issue with discussion and accepted design OR link to spec that describes this work
- [x] Code follows the [standards mentioned here](https://github.com/onflow/flow-cli/blob/master/CONTRIBUTING.md#styleguides)
- [ ] Updated relevant documentation
- [ ] Re-reviewed `Files changed` in the Github PR explorer
- [ ] Added appropriate labels


Patch:
diff --git a/internal/accounts/contract-add.go b/internal/accounts/contract-add.go
index af94b848..8c074f10 100644
--- a/internal/accounts/contract-add.go
+++ b/internal/accounts/contract-add.go
@@ -62,7 +62,10 @@ func addContract(
 		return nil, fmt.Errorf("error loading contract file: %w", err)
 	}
 
-	to := state.Accounts().ByName(addContractFlags.Signer)
+	to, err := state.Accounts().ByName(addContractFlags.Signer)
+	if err != nil {
+		return nil, err
+	}
 
 	account, err := services.Accounts.AddContract(to, name, code, false)
 	if err != nil {
diff --git a/internal/accounts/contract-remove.go b/internal/accounts/contract-remove.go
index 9df6a9d8..4e6093dc 100644
--- a/internal/accounts/contract-remove.go
+++ b/internal/accounts/contract-remove.go
@@ -54,7 +54,11 @@ func removeContract(
 ) (command.Result, error) {
 	contractName := args[0]
 
-	from := state.Accounts().ByName(flagsRemove.Signer)
+	from, err := state.Accounts().ByName(flagsRemove.Signer)
+	if err != nil {
+		return nil, err
+	}
+
 	account, err := services.Accounts.RemoveContract(from, contractName)
 	if err != nil {
 		return nil, err
diff --git a/internal/accounts/contract-update.go b/internal/accounts/contract-update.go
index 61e90ed5..61a7c6a2 100644
--- a/internal/accounts/contract-update.go
+++ b/internal/accounts/contract-update.go
@@ -62,7 +62,10 @@ func updateContract(
 		return nil, fmt.Errorf("error loading contract file: %w", err)
 	}
 
-	to := state.Accounts().ByName(updateFlags.Signer)
+	to, err := state.Accounts().ByName(updateFlags.Signer)
+	if err != nil {
+		return nil, err
+	}
 
 	account, err := services.Accounts.AddContract(to, name, code, true)
 	if err != nil {
diff --git a/internal/accounts/create.go b/internal/accounts/create.go
index 51cdcdab..e1148bb8 100644
--- a/internal/accounts/create.go
+++ b/internal/accounts/create.go
@@ -61,7 +61,10 @@ func create(
 	services *services.Services,
 	state *flowkit.State,
 ) (command.Result, error) {
-	signer := state.Accounts().ByName(createFlags.Signer)
+	signer, err := state.Accounts().ByName(createFlags.Signer)
+	if err != nil {
+		return nil, err
+	}
 
 	sigAlgo := crypto.StringToSignatureAlgorithm(createFlags.SigAlgo)
 	if sigAlgo == crypto.UnknownSignatureAlgorithm {
diff --git a/internal/transactions/build.go b/internal/transactions/build.go
index 7d96b94f..9fcb8101 100644
--- a/internal/transactions/build.go
+++ b/internal/transactions/build.go
@@ -119,9 +119,9 @@ func getAddress(address string, state *flowkit.State) (flow.Address, error) {
 	}
 
 	// if address is not valid then try using the string as an account name.
-	acc := state.Accounts().ByName(address)
-	if acc == nil {
-		return flow.EmptyAddress, fmt.Errorf("account not found, make sure to pass valid account name from configuration or valid flow address")
+	acc, err := state.Accounts().ByName(address)
+	if err != nil {
+		return flow.EmptyAddress, err
 	}
 	return acc.Address(), nil
 }
diff --git a/internal/transactions/send.go b/internal/transactions/send.go
index 5adc5f6d..2c0a64ad 100644
--- a/internal/transactions/send.go
+++ b/internal/transactions/send.go
@@ -59,9 +59,9 @@ func send(
 ) (command.Result, error) {
 	codeFilename := args[0]
 
-	signer := state.Accounts().ByName(sendFlags.Signer)
-	if signer == nil {
-		return nil, fmt.Errorf("signer account: [%s] doesn't exists in configuration", sendFlags.Signer)
+	signer, err := state.Accounts().ByName(sendFlags.Signer)
+	if err != nil {
+		return nil, err
 	}
 
 	code, err := readerWriter.ReadFile(codeFilename)
diff --git a/internal/transactions/sign.go b/internal/transactions/sign.go
index fa12f5a4..b0e62ac0 100644
--- a/internal/transactions/sign.go
+++ b/internal/transactions/sign.go
@@ -59,8 +59,8 @@ func sign(
 		return nil, fmt.Errorf("failed to read partial transaction from %s: %v", filename, err)
 	}
 
-	signer := state.Accounts().ByName(signFlags.Signer)
-	if signer == nil {
+	signer, err := state.Accounts().ByName(signFlags.Signer)
+	if err != nil {
 		return nil, fmt.Errorf("signer account: [%s] doesn't exists in configuration", signFlags.Signer)
 	}
 
diff --git a/pkg/flowkit/account.go b/pkg/flowkit/account.go
index 8ffdbc10..8c2c8de9 100644
--- a/pkg/flowkit/account.go
+++ b/pkg/flowkit/account.go
@@ -134,7 +134,11 @@ type Accounts []Account
 
 // Remove an account.
 func (a *Accounts) Remove(name string) error {
-	account := a.ByName(name)
+	account, err := a.ByName(name)
+	if err != nil {
+		return err
+	}
+
 	if account == nil {
 		return fmt.Errorf("account named %s does not exist in configuration", name)
 	}
@@ -149,25 +153,25 @@ func (a *Accounts) Remove(name string) error {
 }
 
 // ByAddress get an account by address.
-func (a Accounts) ByAddress(address flow.Address) *Account {
+func (a Accounts) ByAddress(address flow.Address) (*Account, error) {
 	for i := range a {
 		if a[i].address == address {
-			return &a[i]
+			return &a[i], nil
 		}
 	}
 
-	return nil
+	return nil, fmt.Errorf("could not find account with address %s in the configuration", address)
 }
 
-// ByName get an account by name.
-func (a Accounts) ByName(name string) *Account {
+// ByName get an account by name or returns and error if no account found
+func (a Accounts) ByName(name string) (*Account, error) {
 	for i := range a {
 		if a[i].name == name {
-			return &a[i]
+			return &a[i], nil
 		}
 	}
+	return nil, fmt.Errorf("could not find account with name %s in the configuration", name)
 
-	return nil
 }
 
 // AddOrUpdate add account if missing or updates if present.
diff --git a/pkg/flowkit/services/project.go b/pkg/flowkit/services/project.go
index fe6a435f..2100f28a 100644
--- a/pkg/flowkit/services/project.go
+++ b/pkg/flowkit/services/project.go
@@ -160,9 +160,9 @@ func (p *Project) Deploy(network string, update bool) ([]*contracts.Contract, er
 			return nil, err
 		}
 
-		targetAccount := p.state.Accounts().ByAddress(contract.Target())
+		targetAccount, err := p.state.Accounts().ByAddress(contract.Target())
 
-		if targetAccount == nil {
+		if err != nil {
 			return nil, fmt.Errorf("target account for deploying contract not found in configuration")
 		}
 
diff --git a/pkg/flowkit/state.go b/pkg/flowkit/state.go
index 7e4fcb15..549b45f1 100644
--- a/pkg/flowkit/state.go
+++ b/pkg/flowkit/state.go
@@ -140,7 +140,7 @@ func (p *State) EmulatorServiceAccount() (*Account, error) {
 		return nil, fmt.Errorf("no default emulator account")
 	}
 
-	return p.accounts.ByName(emulator.ServiceAccount), nil
+	return p.accounts.ByName(emulator.ServiceAccount)
 }
 
 // SetEmulatorKey sets the default emulator service account private key.
@@ -161,9 +161,9 @@ func (p *State) DeploymentContractsByNetwork(network string) ([]Contract, error)
 
 	// get deployments for the specified network
 	for _, deploy := range p.conf.Deployments.ByNetwork(network) {
-		account := p.accounts.ByName(deploy.Account)
-		if account == nil {
-			return nil, fmt.Errorf("could not find account with name %s in the configuration", deploy.Account)
+		account, err := p.accounts.ByName(deploy.Account)
+		if err != nil {
+			return nil, err
 		}
 
 		// go through each contract in this deployment


Test patch:
diff --git a/pkg/flowkit/services/transactions_test.go b/pkg/flowkit/services/transactions_test.go
index 7b1b442c..1e44704a 100644
--- a/pkg/flowkit/services/transactions_test.go
+++ b/pkg/flowkit/services/transactions_test.go
@@ -143,9 +143,9 @@ func TestTransactions_Integration(t *testing.T) {
 			network string
 		}
 
-		a := state.Accounts().ByName("Alice")
-		b := state.Accounts().ByName("Bob")
-		c := state.Accounts().ByName("Charlie")
+		a, _ := state.Accounts().ByName("Alice")
+		b, _ := state.Accounts().ByName("Bob")
+		c, _ := state.Accounts().ByName("Charlie")
 
 		txIns := []txIn{{
 			a.Address(),
diff --git a/pkg/flowkit/state_test.go b/pkg/flowkit/state_test.go
index c7833400..ccb1586b 100644
--- a/pkg/flowkit/state_test.go
+++ b/pkg/flowkit/state_test.go
@@ -344,14 +344,14 @@ func Test_EmulatorConfigSimple(t *testing.T) {
 
 func Test_AccountByAddressSimple(t *testing.T) {
 	p := generateSimpleProject()
-	acc := p.Accounts().ByAddress(flow.ServiceAddress("flow-emulator"))
+	acc, _ := p.Accounts().ByAddress(flow.ServiceAddress("flow-emulator"))
 
 	assert.Equal(t, acc.name, "emulator-account")
 }
 
 func Test_AccountByNameSimple(t *testing.T) {
 	p := generateSimpleProject()
-	acc := p.Accounts().ByName("emulator-account")
+	acc, _ := p.Accounts().ByName("emulator-account")
 
 	assert.Equal(t, flow.ServiceAddress("flow-emulator"), acc.Address())
 	assert.Equal(t, acc.key.ToConfig().PrivateKey, keys()[0])
@@ -423,8 +423,11 @@ func Test_EmulatorConfigComplex(t *testing.T) {
 
 func Test_AccountByAddressComplex(t *testing.T) {
 	p := generateComplexProject()
-	acc1 := p.Accounts().ByAddress(flow.HexToAddress("f8d6e0586b0a20c1"))
-	acc2 := p.Accounts().ByAddress(flow.HexToAddress("0x2c1162386b0a245f"))
+	acc1, err := p.Accounts().ByAddress(flow.HexToAddress("f8d6e0586b0a20c1"))
+
+	assert.NoError(t, err)
+	acc2, err := p.Accounts().ByAddress(flow.HexToAddress("0x2c1162386b0a245f"))
+	assert.NoError(t, err)
 
 	assert.Equal(t, acc1.name, "account-4")
 	assert.Equal(t, acc2.name, "account-2")
@@ -432,7 +435,7 @@ func Test_AccountByAddressComplex(t *testing.T) {
 
 func Test_AccountByNameComplex(t *testing.T) {
 	p := generateComplexProject()
-	acc := p.Accounts().ByName("account-2")
+	acc, _ := p.Accounts().ByName("account-2")
 
 	assert.Equal(t, acc.Address().String(), "2c1162386b0a245f")
 	assert.Equal(t, acc.key.ToConfig().PrivateKey, keys()[1])
@@ -507,7 +510,8 @@ func Test_ChangingState(t *testing.T) {
 	key := NewHexAccountKeyFromPrivateKey(em.Key().Index(), em.Key().HashAlgo(), pk)
 	em.SetKey(key)
 
-	foo := p.Accounts().ByName("foo")
+	foo, err := p.Accounts().ByName("foo")
+	assert.NoError(t, err)
 	assert.NotNil(t, foo)
 	assert.Equal(t, foo.Name(), "foo")
 	assert.Equal(t, foo.Address(), flow.HexToAddress("0x1"))
@@ -516,20 +520,23 @@ func Test_ChangingState(t *testing.T) {
 	assert.NoError(t, err)
 	assert.Equal(t, (*pkey).String(), pk.String())
 
-	bar := p.Accounts().ByAddress(flow.HexToAddress("0x1"))
+	bar, err := p.Accounts().ByAddress(flow.HexToAddress("0x1"))
+	assert.NoError(t, err)
 	bar.SetName("zoo")
-
-	assert.NotNil(t, p.Accounts().ByName("zoo"))
+	zoo, err := p.Accounts().ByName("zoo")
+	assert.NotNil(t, zoo)
+	assert.NoError(t, err)
 
 	a := Account{}
 	a.SetName("bobo")
 	p.Accounts().AddOrUpdate(&a)
+	bobo, err := p.Accounts().ByName("bobo")
+	assert.NotNil(t, bobo)
+	assert.NoError(t, err)
 
-	assert.NotNil(t, p.Accounts().ByName("bobo"))
-
-	p.Accounts().
-		ByName("zoo").
-		SetName("emulator-account")
+	zoo2, _ := p.Accounts().ByName("zoo")
+	zoo2.SetName("emulator-account")
+	assert.Equal(t, "emulator-account", zoo2.name)
 
 	pk, _ = crypto.GeneratePrivateKey(
 		crypto.ECDSA_P256,