py-algorand-sdk icon indicating copy to clipboard operation
py-algorand-sdk copied to clipboard

Python algosdk wallet.Wallet.delete_key() always returns True

Open oysterpack opened this issue 2 years ago • 3 comments

wallet.Wallet.delete_key() always return True

Per the docs, Wallet.delete_key(), should return True if the account has been deleted. Thus, if the wallet does not contain the account, then False is the expected return result. However, Walletdelete_key() always returns True.

Your environment

py-algorand-sdk 2.1.2 algokit 1.0.1

Steps to reproduce

import unittest

from algosdk.wallet import Wallet
from beaker import sandbox


class KmdWalletDeleteKeyTestCase(unittest.TestCase):
    def test_delete_key(self):
        kmd_client = sandbox.kmd.get_client()

        # create new wallet
        name = "foo"
        password = "bar"
        kmd_client.create_wallet(name, password)
        wallet = Wallet(name, password, kmd_client)

        # generate new wallet account
        address = wallet.generate_key()
        self.assertTrue(address in wallet.list_keys())

        # delete wallet account
        self.assertTrue(wallet.delete_key(address))
        self.assertFalse(address in wallet.list_keys())

        # delete wallet account again
        self.assertFalse(
            wallet.delete_key(address),
            "should return False because the wallet does not contain the account",
        ) # this assertion fails


if __name__ == "__main__":
    unittest.main()

Expected behaviour

Wallet.delete_key(address) should return False if the wallet does not contain the specified address

Actual behaviour

Wallet.delete_key(address) always returns True, even if the wallet does not contain the specified address

oysterpack avatar May 01 '23 19:05 oysterpack

Hello, I would like to contribute to it. Can someone please show me the way forward to do it? I am quite a beginner in open source. I can see @algoanne transferred this issue to the GO Language repository, so should I consider looking at that repository for the fix? Please let me know if I am missing some of the vital things to consider which will help me fix the issue.

yusharth avatar Aug 14 '23 10:08 yusharth

Wallet.delete_key(address) should be raised with the error 'Wallet address not found' if the wallet address is wrong.

pasraj avatar Aug 14 '23 13:08 pasraj

Hi @yusharth, I transferred the issue from go-algorand to the python SDK. I believe the ideal fix should be done in this repository and you should not need to worry about go-algorand.

algoanne avatar Aug 16 '23 17:08 algoanne