mikeshultz / ledger-eth-lib

Library to interface with Ethereum app on Ledger hardware wallets
https://ledgereth.readthedocs.io/
MIT License
19 stars 9 forks source link

Default mutable value in transaction model #15

Closed antazoey closed 2 years ago

antazoey commented 2 years ago

We are currently setting an mutable value as a default argument here: https://github.com/mikeshultz/ledger-eth-lib/blob/master/ledgereth/objects.py#L167

This is bad because of reasons like this: https://dollardhingra.hashnode.dev/python-mutable-default-arguments

Basically, the is only one default value, so when you use mutable default values, it is sharing that 1 value around and that can lead some highly unusual outcomes!

antazoey commented 2 years ago

To fix it, you can do something like this:

        access_list: List[bytes] = None,
    ):
        access_list = access_list or []
mikeshultz commented 2 years ago

This is really interesting. I've been working with Python for a very long time but don't think I've ever run into this (or noticed :grimacing:). I'm kind of wondering about all my other implementations of this now...