Skip to content
This repository has been archived by the owner on Nov 30, 2021. It is now read-only.

The api.keyringLock should be unlocked with defer syntax #613

Closed
banishee opened this issue Nov 24, 2020 · 0 comments · Fixed by #614
Closed

The api.keyringLock should be unlocked with defer syntax #613

banishee opened this issue Nov 24, 2020 · 0 comments · Fixed by #614
Assignees
Labels
rpc Ethereum JSON-RPC related issues and PRs Type: Bug

Comments

@banishee
Copy link

System info:
https://github.com/cosmos/ethermint/blob/4501bbccdc8c99532372698d4f48bc66f7de2843/rpc/namespaces/eth/api.go

Steps to reproduce:
I noticed the api code from ethermint/rpc/namespaces/eth/api.go:

func (api *PublicEthereumAPI) Accounts() ([]common.Address, error) {
	api.logger.Debug("eth_accounts")
	api.keyringLock.Lock()

	addresses := make([]common.Address, 0) // return [] instead of nil if empty

	infos, err := api.clientCtx.Keybase.List()
	if err != nil {
		return addresses, err
	}

	api.keyringLock.Unlock()

	for _, info := range infos {
		addressBytes := info.GetPubKey().Address().Bytes()
		addresses = append(addresses, common.BytesToAddress(addressBytes))
	}

	return addresses, nil
}

I suppose that this is not an appropriate way of unlocking a lock.
From my opinion, the keyringLock might not to be unlocked if the api.clientCtx.Keybase.List() cause an error
Or is there something special reason for not using defer to unlock in this function?

Additional info:
I am not sure it is a bug or not, and I just want to make it clear

@fedekunze fedekunze self-assigned this Nov 24, 2020
@fedekunze fedekunze added rpc Ethereum JSON-RPC related issues and PRs Type: Bug labels Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
rpc Ethereum JSON-RPC related issues and PRs Type: Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants