Skip to content

WalletRegistry generates 33-character public keys that fail the platform's own address validator #365

Description

@usmanimamu17-create

Problem

WalletRegistry._build_public_key (app/services/wallet_registry.py):

@staticmethod
def _build_public_key() -> str:
    return f"G{uuid4().hex.upper()}"

uuid4().hex is 32 chars, so the result is 33 characters total. Stellar public keys are exactly 56 chars, and wallet_address.normalize (app/utils/wallet_address.py) rejects anything not exactly 56 (Stellar public keys must be exactly 56 characters (got 33)).

Consequences:

  • Every wallet created without a user-supplied key gets an invalid key: create_wallet stores a 33-char string in WalletORM.public_key (a String(56) unique column), so the platform's own data violates its validator; any code path that later validates the stored key (GDPR export, linking, funding checks) fails or misbehaves.
  • Keys are not unique in any real sense: uuid4().hex is random but the string is not a Stellar key at all; get_balance/get_status treat funded/trustline_ready flags as authoritative, so the fake key never reaches the network — but the moment a real Horizon fetch is added (tracked separately), every existing row is invalid.
  • The checksum validator (tracked separately) will make this worse: generation and validation currently disagree, and no test asserts the generated key satisfies wallet_address.is_valid.

Root cause

The generator was written as a placeholder ("G" + hex) and never reconciled with the address grammar the rest of the codebase enforces.

Why this is architecturally hard

  1. Generating a real Stellar key pair requires an ed25519 keypair (e.g. the stellar-sdk key generation) and deriving the G... address — a dependency/design decision the repo has not made; alternatively, wallet creation should require a client-supplied key validated by wallet_address.normalize, removing the server-side generator.
  2. Existing rows with 33-char keys must be handled: a migration to invalidate/repair them or a documented deprecation of the generated-key path.
  3. The create-wallet response message ("Please fund with at least 1 XLM") implies a real network account; if keys are simulated, the response should say so explicitly (same class of issue as the simulated balances).

Proposed design

Either generate real keypairs (via stellar-sdk or a documented KMS flow) or require validated client-supplied keys in WalletCreateRequest; add a test asserting every stored public_key passes wallet_address.is_valid, and a data check for existing rows.

Acceptance criteria

Service

  • Every wallet public_key stored passes wallet_address.is_valid.
  • Create-wallet responses are explicit about simulation vs real keys.

Tests

  • A test asserts generated keys validate.
  • Existing wallet tests pass.

Out of scope

CRC16 checksum validation (tracked separately) and simulated balances (tracked separately).

Getting started

pytest tests/test_wallet_persistence.py -q
make typecheck

Good first files to read: app/services/wallet_registry.py, app/utils/wallet_address.py, app/models/orm/wallet.py.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardThird CampaignCampaign: Third Campaignarea/walletsImported campaign issue labelpriority/highImportant; address in current quarter

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions