docs(skill): guide testnet and mainnet funding - #34
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60c2bcd6d2
ℹ️ 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".
snissn
left a comment
snissn
left a comment
There was a problem hiding this comment.
Codex review — changes required before merge
Reviewed on exact head 60c2bcd6d2187824dc7fc1a875b706d6fca8f361.
The funding sequence can unnecessarily require additional USDFC and overfund an existing account. Both guides should establish the actual upload size/runway/copies, inspect wallet and Filecoin Pay state, and run wallet costs before requiring wallet USDFC. alreadyCovered and depositNeeded are the authoritative answer about whether more payment funding is needed.
Concretely:
- On mainnet, do not require both assets to “arrive” when
availableFundsalready covers the quoted upload; request only missing FIL for gas and any USDFC shortfall actually identified by the quote. - Apply the same rule to Calibration. The current “After wallet tFIL and tUSDFC are present” gate still blocks a wallet whose Filecoin Pay account is already sufficiently funded.
- Keep wallet USDFC, total Pay funds, available Pay funds, and required deposit distinct in the final decision, not only in the displayed balance summary.
The existing unresolved inline thread identifies the mainnet instance correctly. The documented faucet URLs match the current official Filecoin resources; I found no link blocker.
GitHub does not permit a reviewer authenticated as the PR author to submit a formal REQUEST_CHANGES review, so this is posted as a blocking comment review.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: caa77ee750
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c3aa5519e6
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08faf2af4d
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0933bc234a
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c4f786730
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Resolved on current head
All three requested changes are present on the current PR head. No merge performed. |
|
Merge #35 before this. It replaces FUND_FAILED with per-asset funded / missing / unconfirmed statuses, and step 4 of testnet-funding.md still describes the old behavior. After #35 lands, rewrite step 4 to read those status fields instead of inferring outcomes from raw balances. Everything else looks good. |
|
@nijoe1 ready for re-review on exact head
Validation on this combined head: |
There was a problem hiding this comment.
Pull request overview
This PR updates the foc-cli “skill” documentation to provide clearer, deterministic funding guidance for both Calibration testnet and mainnet, emphasizing balance checks, separation of wallet vs Filecoin Pay funds, and safe faucet/browser handoffs.
Changes:
- Updates the main SKILL guide to point to a dedicated Calibration funding reference with balance-checking guidance.
- Adds a new Calibration (testnet) funding workflow describing when to use
wallet fundvs browser-based faucet fallbacks and how to interpret per-asset statuses. - Expands the mainnet funding reference with an explicit agent workflow (balance → costs → shortfall-only acquisition guidance → confirm → deposit/upload gating).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| skills/foc-cli/SKILL.md | Updates funding section to link to the new testnet funding reference and clarify mainnet workflow reference. |
| skills/foc-cli/references/testnet-funding.md | New Calibration funding guide with balance-first workflow and safe faucet fallback ladder. |
| skills/foc-cli/references/mainnet-funding.md | Adds a step-by-step mainnet workflow emphasizing shortfall-only funding guidance and conditional deposit/upload steps. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Summary
Review convergence
The initial internal review found three issues: an unsafe mint fallback for auto wallets, a possible duplicate claim after an uncertain zero balance, and a repeated funding-owner question. All were fixed and re-reviewed cleanly.
Codex findings were resolved in successive rounds. Both network flows now check existing payment-account funds before requesting wallet USDFC. The Calibration flow avoids combined claims for partially funded wallets and treats any zero after a combined claim as unconfirmed until rechecked. The mainnet flow skips
wallet depositwhendepositNeededis zero, labelswallet costsas an initial estimate, and carries the quoted copy/CDN settings into upload. All threads are resolved.Validation
bun run test: 203 passed, 500 assertionsbunx biome check src testsbunx tsc --noEmitbun run buildScope
Documentation and skill routing only. No CLI behavior changes.