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.

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

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

Open
oysterpack opened this issue May 1, 2023 · 3 comments
Open

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

oysterpack opened this issue May 1, 2023 · 3 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@oysterpack
Copy link

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 oysterpack added the bug Something isn't working label May 1, 2023
@algorand algorand deleted a comment May 1, 2023
@algoanne algoanne transferred this issue from algorand/go-algorand May 2, 2023
@algoanne algoanne added the good first issue Good for newcomers label May 22, 2023
@yusharth
Copy link

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.

@pasraj
Copy link

pasraj commented Aug 14, 2023

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

@algoanne
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants