Skip to content

Conversation

@0xRAG
Copy link
Contributor

@0xRAG 0xRAG commented Dec 18, 2024

What changed? Why?

It should be clear which saved wallets belong to which networks when you save_seed / export_data for a given wallet.

How i tested

After running:

# Create a new wallet
wallet = Wallet.create(network_id="base-mainnet")

print(wallet.export_data())

# Save the wallet seed
wallet.save_seed("/Users/ryan/code/public/seeds/wallet_seed.json")

wallet_seed.json looks like:

{
  "11cd62ab-0046-472d-bb16-3ad560bdf2d4": {
        "seed": "21exxx",
        "encrypted": false,
        "auth_tag": "",
        "iv": "",
        "network_id": "base-mainnet"
    }
}

And the result of wallet.export():

WalletData: (wallet_id: 11cd62ab-0046-472d-bb16-3ad560bdf2d4, seed: 21exxx, network_id: base-mainnet)

Qualified Impact

The network id should always be present, and if it is not, it will simply not be included in the WalletData

CHANGELOG.md Outdated
### Added

- Wallet address contract invocation input validation for payable contracts.
- Add `network_id` to `WalletData` so that it is saved with the seed data and surfaced via the export function
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add new changes under Unreleased heading until the new release is being cut

@0xRAG 0xRAG changed the title chore: add network_id to WalletData chore(PSDK-640): add network_id to WalletData Dec 18, 2024
@0xRAG 0xRAG marked this pull request as ready for review December 18, 2024 16:07
return self._seed

@property
def network_id(self) -> str:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return type can be None -> str | None

@0xRAG 0xRAG merged commit 7f4780d into v0.13.0 Dec 18, 2024
7 checks passed
@0xRAG 0xRAG deleted the PSDK-640 branch December 18, 2024 21:41
@derek-cb derek-cb mentioned this pull request Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants