docs: Ethereum integration guide - #39
Conversation
Review: Ethereum integration guideMust fix
Suggestions
Verified
|
|
Please review before merging. |
bffdf77 to
1945174
Compare
1945174 to
3795697
Compare
|
Feedback addressed:
|
…I guidance - Fix deprecated Rust ECDSA import: ic_cdk::api::management_canister::ecdsa -> ic_cdk::management_canister; EcdsaPublicKeyArgument -> EcdsaPublicKeyArgs - Replace dfx_test_key with test_key_1 as default (both Motoko and Rust); mention key_1 for production - MultiResult/RpcResult -> MultiRpcResult with correct Ok/Err arms throughout all Rust snippets; use evm_rpc_types imports - Update prose reference: MultiResult -> MultiRpcResult - CLI testing: replace unresolved verification comment with clear guidance — query calls work directly, update calls require going through backend wrapper - Remove local deployment verification comment (resolved by icp.yaml approach) - Cargo.toml note: remove unused evm_rpc_client and ic-canister-runtime; keep only evm_rpc_types and ic-cdk which the examples actually use
|
Review complete. All findings addressed in commit 6faab6b: Fixed:
Verified:
|
…date Upstream comment
|
Feedback addressed:
|
## Summary Follow-up to #376. That PR correctly retargeted the opt-in caution from the old single terms of service to the [App Operator Terms](https://internetcomputer.org/icp-mcp/app-operator-terms/), but it inherited the ambiguity those Terms carried: it presented registration as the acceptance step while also treating manifest publication as acceptance by conduct, so a reader could not tell what they must do before their app can be acted on. dfinity/internetcomputer-org#96 resolved that in the Terms themselves. This states the same model here, which matters because the Terms point at this page for how acceptance happens (and say that where the two differ, the Terms prevail). ## What the caution now says - **Publishing the Layer 1 manifest at `/.well-known/ic-architecture` opts the app in and constitutes the operator's acceptance** of the App Operator Terms. It is sufficient to make the app's declared canisters eligible for everything ICP MCP does with them, queries and state-changing calls alike; no separate registration is required. - **Publication is the condition ICP MCP applies to every call that reaches a canister.** It reads and calls only canisters a manifest declares, so an app that publishes none is still resolved and described, but none of its canisters is read or called. Removing the manifest stops new calls of every kind. - **Registration is optional** and is not a condition of participation. It is how an operator tells DFINITY who they are, so they can be reached with the notices the terms provide for, and so there is a record of who accepted and which version; it can carry the URL of the app's **own** privacy policy so ICP MCP can present it to users. - The privacy sentence describes what the [ICP MCP Privacy Policy](https://internetcomputer.org/icp-mcp/privacy-policy/) actually covers (what is disclosed to a participating app, and what registration processes), and states that the operator remains responsible for handling that data lawfully and for keeping the app's own privacy notice accurate — rather than asking operators to "comply with" a notice that describes DFINITY's processing and expressly does not govern theirs. - The **caution title** now names the trigger ("Publishing the Layer 1 manifest opts your app in"), which review caught contradicting the body: the old title read as though any layer opts an app in, while the body pins opt-in to Layer 1 and notes the layers are otherwise independently adoptable. The five-layer content below the caution is unchanged. ## Revision history This PR went through three models of what publication gates, because the code moved under it: 1. An early revision gated state-changing calls on DFINITY confirming the operator's registration. That was the wrong branch of the two the review offered; publication really was sufficient, so registration became optional instead. 2. Two review suggestions then narrowed what publication gates to **state-changing calls only**, on the correct ground that `crates/imcp2-core/src/discoverability.rs` gated writes only and its `READS_ARE_FINE` refusal text told agents that reads worked regardless. Both were applied verbatim in `60a73a0` and `8ec933f`. 3. **`e4ce4be`** undoes that narrowing, because dfinity/imcp2#184 (`64c48be`, "Gate every canister-reaching tool on the discoverability manifest") has since made reads gated the same way. `get_canister_candid`, `get_canister_api_doc`, `get_canister_oql_schema`, `canister_query` on both paths and `canister_update_call` are all restricted to declared canisters; per the module docs, "Reads and writes share one mechanism and differ only in what a refusal SAYS", `the_rule()` now says the server "**reads and calls** ONLY canisters an app declares", and `READS_ARE_FINE` is gone. So the two suggested sentences were accurate when written and are not now. Step 3 keeps their substance where it still holds (publication is sufficient, registration is optional) and moves the line to where the code now draws it: not reads versus writes, but **calls that reach a canister versus the surface that does not**. `open_app`, the static guides and the identity tools reach no canister and stay ungated, which is why the caution still says an app publishing no manifest is resolved and described. @sea-snake, your approval was on `8ec933f`, so this last commit is newer than what you read. ## Verification - `npm run validate` — **210 files, all checks passed** on each revision, including `e4ce4be` (the repo's no-em-dash rule caught new prose in the first draft, fixed before pushing). - Rendered preview checked, not just the source, on the earlier revisions: the caution reads as intended and the page links only `app-operator-terms/` and `privacy-policy/`, with no `/icp-mcp/terms/` link that would send operators to the User Terms. - The claims about the gate are quoted from `crates/imcp2-core/src/discoverability.rs` at imcp2 `main` (`64c48be`), not from memory. ## Companion dfinity/internetcomputer-org#97 makes the matching correction to App Operator Terms §3, §4 and §7. Merge that with or before this, since the Terms are the authority this page defers to. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01Xpg83AWtkQo34jbMxpX8WB --------- Co-authored-by: Claude <noreply@anthropic.com>
Summary
Sync recommendation
Informed by
dfinity/portal—docs/building-apps/chain-fusion/ethereum/*;dfinity/icskills—skills/evm-rpc/SKILL.md;dfinity/cdk-rs—ic-cdk/src/management_canister.rs(ECDSA management canister API)