Skip to content

fix: 3 critical bugs — Permit2 allowance threshold, Firebase token refresh, create_model notes mismatch#202

Open
amathxbt wants to merge 3 commits intoOpenGradient:mainfrom
amathxbt:fix/opg-token-allowance-modelhub-firebase-refresh
Open

fix: 3 critical bugs — Permit2 allowance threshold, Firebase token refresh, create_model notes mismatch#202
amathxbt wants to merge 3 commits intoOpenGradient:mainfrom
amathxbt:fix/opg-token-allowance-modelhub-firebase-refresh

Conversation

@amathxbt
Copy link
Contributor

Summary of Changes

This PR fixes 3 critical bugs found across opg_token.py and model_hub.py, directly addressing team-reported issues #188, #164, and #157.


Bug 1 — opg_token.py: Permit2 allowance threshold uses 10% instead of 100% (relates to #188)

File: src/opengradient/client/opg_token.py

Problem: The guard used 0.1 * amount_base (10%) instead of the full amount. Any wallet with 10–99% of the required OPG approved would skip the approval step and then fail the x402 payment with an HTTP 402 error downstream — the same symptom reported in issue #188.

# BEFORE (broken)
if allowance_before >= amount_base * 0.1:

# AFTER (correct)
if allowance_before >= amount_base:

Bug 2 — model_hub.py: Firebase idToken expires after 1 hour causing silent 401s (closes #164)

File: src/opengradient/client/model_hub.py

Problem: ModelHub stored the Firebase idToken at login and never refreshed it. Firebase tokens expire after 3600 seconds. Any API call made more than ~1 hour after constructing a ModelHub instance fails silently with 401 Unauthorized.

Fix: Added _get_auth_token() which checks time against a stored expiry timestamp and calls firebase_app.auth().refresh(refreshToken) when within 60s of expiry. All methods now call _get_auth_token() instead of accessing idToken directly.


Bug 3 — model_hub.py: create_model() passes version label as notes to create_version() (relates to #157)

File: src/opengradient/client/model_hub.py

Problem: create_version(model_name, notes='', is_major=False) — the version string was mapped to the notes positional parameter. The server auto-assigns its own version string, so the intent was silently lost and release notes said '1.00' instead of anything descriptive. The local variable model_name was also shadowed.

Fix:

# AFTER: explicit keyword arg + renamed local variable
version_response = self.create_version(created_name, notes=f'Initial version {version}')

Files Changed

  • src/opengradient/client/opg_token.py — Fix allowance threshold
  • src/opengradient/client/model_hub.py — Add token auto-refresh + fix create_model notes

Linked Issues

cc @adambalogh — please review when you get a chance!

…nGradient#188)

The allowance guard used amount_base * 0.1 (10%) which allowed
x402 payments to proceed when only 10% of the required OPG was
approved. This caused downstream payment failures when the
allowance was between 10-99% of the required amount.

Fix: compare allowance_before against the full amount_base so
the approval step is skipped only when allowance is already
sufficient.
…closes OpenGradient#164, OpenGradient#157)

Bug 1 — Firebase idToken expiry (closes OpenGradient#164):
  ModelHub cached self._hub_user at login time and never refreshed
  the idToken.  Firebase tokens expire after 3600 s, so any API call
  made more than ~1 hour after construction silently fails with 401.
  Fix: add _get_auth_token() which checks time.time() against a stored
  expiry and calls firebase_app.auth().refresh(refreshToken) when the
  token is within _TOKEN_REFRESH_MARGIN_SEC (60 s) of expiry.  All
  methods now call _get_auth_token() instead of reading idToken directly.

Bug 2 — create_model passes version label as notes (closes OpenGradient#157):
  create_model(model_name, model_desc, version='1.00') called
  self.create_version(model_name, version) which mapped the version
  string '1.00' to the  positional parameter of create_version.
  The server ignores that field as a version specifier and auto-assigns
  its own version string, so the argument was silently discarded.
  Fix: call self.create_version(created_name, notes=f'Initial version {version}')
  to make the intent explicit, and rename the local variable from
  model_name to created_name to avoid shadowing the input parameter.
@amathxbt
Copy link
Contributor Author

amathxbt commented Mar 23, 2026

Hey @adambalogh
here's PR #202 with 3 more critical bug fixes found while auditing the SDK codebase. All three directly relate to open team issues:


Bug 1 — opg_token.py: Permit2 allowance threshold was 10% not 100% (related to #188)
The early-return guard if allowance_before >= amount_base * 0.1 meant any wallet with 10–99% of the required OPG allowance silently skipped the approval and then hit an HTTP 402 when actually paying. Changed to >= amount_base (full coverage check).


Bug 2 — model_hub.py: Firebase idToken never refreshed → silent 401 after 1 hour (closes #164)
ModelHub cached the Firebase idToken at login but never refreshed it. Firebase tokens expire after 3600 s. Added _get_auth_token() with auto-refresh logic using refreshToken; all methods now call it instead of reading idToken directly.


Bug 3 — model_hub.py: create_model() passed version string as release notes (related to #157)
self.create_version(model_name, version) mapped the version="1.00" parameter to the notes positional arg of create_version(). The server auto-assigns the version number anyway, so the label was silently discarded. Fixed to use the explicit keyword: notes=f"Initial version {version}" and renamed the shadowed local variable.


Files changed: opg_token.py, model_hub.py — 2 commits, no breaking API changes. Please let me know if you'd like any changes!

firebase is an untyped package (type: ignore[import-untyped]), so
self._hub_user['idToken'] resolves to Any. Since _get_auth_token()
is declared -> str, mypy raised:
  error: Returning Any from function declared to return 'str' [no-any-return]

Fix: wrap with str() cast which is always safe since Firebase idTokens
are JWT strings.
@amathxbt
Copy link
Contributor Author

amathxbt commented Mar 24, 2026

@adambalogh
pushed a fix for the CI check failure just now 🔧

Error from uv run mypy src:

src/opengradient/client/model_hub.py:83: error: Returning Any from function declared to return "str" [no-any-return]

Root cause: firebase is an untyped package (type: ignore[import-untyped]), so self._hub_user["idToken"] resolves to Any. Since _get_auth_token() is declared -> str, mypy flagged the bare return as [no-any-return].

Fix (commit ea3b99c):

# Before
return self._hub_user["idToken"]

# After
return str(self._hub_user["idToken"])  # cast Any->str for mypy [no-any-return]

The str() cast is always safe — Firebase idTokens are JWT strings at runtime. This should clear the only remaining CI error while keeping all other checks passed

@amathxbt
Copy link
Contributor Author

@adambalogh — PR #202 is fully ready to merge! 🚀

All CI checks are green ✅:

  • check (mypy + ruff) ✅
  • test
  • e2e
  • CodeFactor

The PR is mergeable with no conflicts. As a contributor I don't have write access to merge into main directly — could you please review and merge when you get a chance? Thank you! 🙏

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fire base API key

1 participant