Repository navigation
docs: ADR 0001 FWSS fork, designs for #618 and #619 - #1
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f86acc39a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| The naming follows upstream's only FWSS precedent, PR #615. | ||
| 5. **Agree the layout before mainnet.** The namespace ids and structs, the extraData tuples, the EIP-712 type strings and the session-key permissions are much harder to change once data sets exist: a change after deploy needs a one-time migration in an upgrade. FF has asked upstream to confirm them in #618 and #619. | ||
| 6. **End condition.** FF's proxy upgrades to an upstream release that contains equivalent code, and the fork is retired. | ||
| 7. **Fallback.** If upstream's released layout differs from FF's, the upgrade to that release carries one bridging `migrate()` that moves FF's namespaced state into upstream's layout. It runs once. |
There was a problem hiding this comment.
Make the fallback migration incremental
If upstream chooses a different layout after this deployment has accumulated many data sets, a single migrate() cannot safely move all pricing records: FWSSDataSetPricingStorage is an unbounded mapping with no key enumeration, and scanning data-set IDs plus rewriting every entry can exceed the transaction gas limit. This can block the stated convergence upgrade, so define a batched or lazy per-data-set migration with a version marker before relying on this fallback.
Useful? React with 👍 / 👎.
Summary
main, so they resolve once this merges.Rendered:
🤖 Generated with Claude Code