Skip to content

feat: add transfer-assets-into-polkadot polkadot-docs test harness#288

Merged
nhussein11 merged 4 commits into
masterfrom
feat/polkadot-docs-transfer-assets-into-polkadot
May 4, 2026
Merged

feat: add transfer-assets-into-polkadot polkadot-docs test harness#288
nhussein11 merged 4 commits into
masterfrom
feat/polkadot-docs-transfer-assets-into-polkadot

Conversation

@nhussein11

Copy link
Copy Markdown
Contributor

Summary

Adds a test harness verifying the Transfer Assets from Ethereum into Polkadot guide. Tests cover getSupportedAssets for Ethereum→AssetHubPolkadot and Ethereum→Hydration routes, the EXTERNAL_CHAINS constant, and EvmBuilder construction; live bridge submission is guarded by it.skipIf(!ETH_PRIVATE_KEY).

Test plan

  • cargo fmt --check --package sdk
  • cargo clippy --package sdk --locked -- -D warnings
  • cargo build --workspace --locked
  • Harness tests pass locally (if touched): cd <harness> && npm ci && npm test
  • /check-internal-docs passes (if touching versions, docs, badges)
  • /branding re-run (if touching .github/brand/tokens.yml or generated assets)
  • CI is green on this PR

Checklist

  • Branch is NOT master
  • Commits follow Conventional Commits (feat:, fix:, chore:, …)
  • No secrets, credentials, or .env files committed
  • No hardcoded hex colors outside .github/brand/tokens.yml (see DESIGN.md)
  • Draft PR if generated by a skill (--draft)

Adds a test harness verifying the Transfer Assets from Ethereum into Polkadot guide.
Tests cover getSupportedAssets for Ethereum→AssetHubPolkadot and Ethereum→Hydration
routes, the EXTERNAL_CHAINS constant, and EvmBuilder construction; live bridge
submission is guarded by it.skipIf(!ETH_PRIVATE_KEY).
@nhussein11 nhussein11 self-assigned this Apr 28, 2026

@brunopgalvao brunopgalvao left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few things before this is good to merge:

  1. CI uses npm install. Should be npm ci — the harness README tells users npm ci, most sibling polkadot-docs-*.yml workflows do too, and npm install can mutate the lockfile mid-run. One-line fix.

  2. Test 4's Hydration branch silently passes. If WETH isn't supported on Hydration, the test bails with a bare return and zero assertions, so it stays green even if the builder breaks. Flip it to it.skipIf(!weth) or assert something before returning.

  3. Live test calls .build() but the README says "sign + submit". In most paraspell builders .build() returns the constructed tx without broadcasting. Either the test needs the actual submit method or the README needs to say "build" — please confirm against the SDK and pick one. Right now they're out of sync.

  4. @polkadot/api declared but never imported. Drop it from dependencies if it's only there transitively via @paraspell/sdk-pjs, otherwise add the code that uses it. Dead deps just add lockfile noise.


Style nit (not blocking): await import("ethers") is unnecessary since ethers is a top-level dep — just import it statically at the top of the file. Dynamic imports are for the PAPI-style "load only if used" cases.

Also worth ticking off the test-plan boxes in the PR description as items are verified — per CLAUDE.md they shouldn't ship unchecked.

Happy to approve once 1–4 are sorted.

@nhussein11 nhussein11 requested a review from brunopgalvao April 29, 2026 12:16
brunopgalvao
brunopgalvao previously approved these changes Apr 30, 2026

@brunopgalvao brunopgalvao left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All 4 + the style nit addressed cleanly — , on Hydration, README/code aligned on what .build() does, and the unused @polkadot/api drop. CI green. Quick reminder to tick the test-plan boxes in the PR body before merge (CLAUDE.md). Nice work.

@brunopgalvao brunopgalvao left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

@brunopgalvao brunopgalvao dismissed their stale review April 30, 2026 04:17

Duplicate — body got mangled by shell quoting; superseded by the next approval review with the intended text.

@nhussein11 nhussein11 merged commit 8a5f738 into master May 4, 2026
8 checks passed
@nhussein11 nhussein11 deleted the feat/polkadot-docs-transfer-assets-into-polkadot branch May 4, 2026 12:09
@github-actions github-actions Bot mentioned this pull request May 11, 2026
3 tasks
@github-actions github-actions Bot mentioned this pull request May 18, 2026
3 tasks
@brunopgalvao brunopgalvao mentioned this pull request May 21, 2026
9 tasks
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.

2 participants