Skip to content

Commit 09af6a8

Browse files
marc0oloclaude
andauthored
chore: migrate motoko/icrc2-swap to icp-cli (#1386)
* chore: migrate motoko/icrc2-swap to icp-cli Replace dfx.json with icp.yaml, migrate Motoko sources from src/swap/ to backend/, add icp-dev-env-motoko:0.3.2 CI workflow, update mops.toml to moc 1.9.0 / core 2.5.0 with --default-persistent-actors. Remove JS/Jest test suite, old deploy.sh, and prettier config; add Makefile with deploy and test targets that handle the multi-step deployment (token_a, token_b ICRC-1 ledger canisters via pre-built WASM, then backend swap canister with dynamic init args containing the token canister IDs). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(icrc2-swap): remove old Motoko workflow and Candid section from README Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(icrc2-swap): add --query flag to query canister calls Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(icrc2-swap): remove GitHub link and Overview heading from README Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(icrc2-swap): fix deploy/test commands and improve example quality Deploy: - --argument → --args (correct icp-cli flag) - icp canister id → icp canister status X -i Tests: - Replace single-user contrived test with real two-user scenario: Alice deposits token_a, Bob deposits token_b, they swap 1:1, each withdraws the other's token - Use --identity test-alice/test-bob for two distinct callers README: - Credit original author 0xAegir (AegirFinance) - Remove "DeFi is experimental" disclaimer - Fix security link: old URL → docs.internetcomputer.org/guides/security/inter-canister-calls - Note that make deploy must be used instead of icp deploy - Explain what make test actually tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(icrc2-swap): clarify why icp deploy alone fails — make deploy required Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(icrc2-swap): use Runtime.envVar for token discovery, remove init args - Change from actor class Swap(init_args) to plain actor Swap - Token principals read from PUBLIC_CANISTER_ID:token_a / token_b via Runtime.envVar<system>() — injected automatically by icp-cli during deploy - Remove --args from make deploy backend step; no init args needed - Backend deploys last so icp-cli has both token IDs to inject - Makefile deploy and test use correct icp-cli flags (--args, status -i) - M0155 (Nat subtraction may trap) intentionally left as a warning — subtraction is guarded by explicit balance check but Motoko's type checker lacks path-sensitive refinement Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(icrc2-swap): pre-fund named identities, no minting needed in tests - Create icrc2-alice and icrc2-bob identities in make deploy (idempotent) - token_a initial_balances → icrc2-alice; token_b → icrc2-bob - No funding/minting step needed in make test — identities start with tokens - Tests use --identity icrc2-alice/icrc2-bob for realistic two-user flow - README updated to explain the named identities and env var discovery Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(icrc2-swap): use single shell block in make deploy to fix ordering $(eval ALICE := $(shell ...)) in Make recipe bodies expands before any recipe commands run — so icp identity principal ran before icp identity new. Fix: use a single shell block with set -e so identity creation and principal lookup happen in the correct order within one shell process. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(icrc2-swap): DX review — clean up, improve README, fix tests .gitignore: remove stale token_a.args/token_b.args/backend.args entries from the old dfx arg-file approach Makefile: - Tests 5/6 use single shell block to capture balance delta (before/after) making them idempotent across multiple make test runs - Fee arithmetic comments added to approve/withdraw steps README: - Lead with the two key safety patterns (debit-before-transfer, atomic swap) which are the core educational content — previously buried in code comments - Add ic-mops to prerequisites - Explain ICRC.mo inline type definitions - Add fee handling section explaining the approve/withdraw arithmetic - Clarify make deploy vs icp deploy and why - Tighten known limitations section Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(icrc2-swap): remove redundant .gitignore — covered by root Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(icrc2-swap): clarify "perform the transfer on the token ledger" Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(icrc2-swap): security review — try/catch on withdraw, anonymous rejection From canister-security skill review: Pitfall #11 (callback trap after state mutation): - withdraw: balance is debited before await icrc1_transfer - if the token ledger TRAPS (not just returns an error), the callback never ran and the refund in the #Err branch is skipped - Fix: wrap await icrc1_transfer in try/catch; on trap, refund and return #CallFailed with the error message - Add #CallFailed error variant to WithdrawError Pitfall #2 (anonymous principal): - withdraw uses msg.caller as balance key but never rejected anonymous - Fix: trap on anonymous caller at entry README known limitations updated to accurately describe: - the async edge case for trusted-only token canisters (not "deadlock") - the deposit ambiguous-outcome scenario Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(icrc2-swap): clarify #CallFailed retry guidance and ledger operator escalation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(icrc2-swap): check non-zero balances before swap, add error test swap() now returns InsufficientBalance if either user has no deposit — previously it silently succeeded moving zero tokens. Add Test 2 (before any deposits) to verify InsufficientBalance fires, making the guard visible in the test output. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(icrc2-swap): remove "on-chain" — replace with clearer phrasing Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(icrc2-swap): link ICRC-2 to digital asset standards reference Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(icrc2-swap): use --storage plaintext for identity creation in CI The default --storage keyring requires a system keyring unavailable in CI containers. --storage plaintext stores the key as a plain PEM file, which works in both CI and local dev. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 96c5861 commit 09af6a8

24 files changed

Lines changed: 313 additions & 12043 deletions

.github/workflows/icrc2-swap.yml

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
name: icrc2-swap
2+
3+
on:
4+
push:
5+
branches: [master]
6+
pull_request:
7+
paths:
8+
- motoko/icrc2-swap/**
9+
- .github/workflows/icrc2-swap.yml
10+
11+
concurrency:
12+
group: ${{ github.workflow }}-${{ github.ref }}
13+
cancel-in-progress: true
14+
15+
jobs:
16+
motoko-icrc2-swap:
17+
runs-on: ubuntu-24.04
18+
container: ghcr.io/dfinity/icp-dev-env-motoko:0.3.2
19+
env:
20+
ICP_CLI_GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
21+
steps:
22+
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4.3.1
23+
- name: Deploy and test
24+
working-directory: motoko/icrc2-swap
25+
run: |
26+
icp network start -d
27+
make deploy
28+
make test

.github/workflows/motoko-icrc2-swap-example.yml

Lines changed: 0 additions & 29 deletions
This file was deleted.

motoko/icrc2-swap/.gitignore

Lines changed: 0 additions & 34 deletions
This file was deleted.

motoko/icrc2-swap/.prettierignore

Lines changed: 0 additions & 3 deletions
This file was deleted.

motoko/icrc2-swap/.prettierrc

Lines changed: 0 additions & 3 deletions
This file was deleted.

motoko/icrc2-swap/Makefile

Lines changed: 139 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,141 @@
1-
.PHONY: test
1+
.PHONY: deploy test
22

3+
# Deploy all canisters in the correct order:
4+
# 1. Create example-specific test identities (icrc2-alice, icrc2-bob) if not present.
5+
# 2. Deploy token_a (pre-funded for alice) and token_b (pre-funded for bob).
6+
# 3. Deploy backend — discovers token principals automatically via env vars.
7+
#
8+
# Note: use `make deploy` (not `icp deploy`) — the ledger canisters require
9+
# init args with alice/bob principals that are only available after the
10+
# identities are created.
11+
deploy:
12+
@set -e; \
13+
MINTER=$$(icp identity principal); \
14+
icp identity new icrc2-alice --storage plaintext 2>/dev/null || true; \
15+
icp identity new icrc2-bob --storage plaintext 2>/dev/null || true; \
16+
ALICE=$$(icp identity principal --identity icrc2-alice); \
17+
BOB=$$(icp identity principal --identity icrc2-bob); \
18+
icp deploy token_a --mode reinstall -y --args "(variant { Init = record { \
19+
token_name = \"Token A\"; token_symbol = \"A\"; \
20+
minting_account = record { owner = principal \"$$MINTER\" }; \
21+
initial_balances = vec { record { record { owner = principal \"$$ALICE\" }; 100_000_000_000_000 : nat } }; \
22+
metadata = vec {}; transfer_fee = 10_000 : nat; \
23+
archive_options = record { trigger_threshold = 2000 : nat64; num_blocks_to_archive = 1000 : nat64; controller_id = principal \"$$MINTER\" }; \
24+
feature_flags = opt record { icrc2 = true } } })"; \
25+
icp deploy token_b --mode reinstall -y --args "(variant { Init = record { \
26+
token_name = \"Token B\"; token_symbol = \"B\"; \
27+
minting_account = record { owner = principal \"$$MINTER\" }; \
28+
initial_balances = vec { record { record { owner = principal \"$$BOB\" }; 100_000_000_000_000 : nat } }; \
29+
metadata = vec {}; transfer_fee = 10_000 : nat; \
30+
archive_options = record { trigger_threshold = 2000 : nat64; num_blocks_to_archive = 1000 : nat64; controller_id = principal \"$$MINTER\" }; \
31+
feature_flags = opt record { icrc2 = true } } })"; \
32+
icp deploy backend --mode reinstall -y
33+
34+
# Full integration test: icrc2-alice deposits token_a, icrc2-bob deposits token_b,
35+
# they swap 1:1, and each withdraws the other's token.
336
test:
4-
./test/deploy.sh
5-
dfx generate
6-
npm run test
37+
$(eval TOKEN_A := $(shell icp canister status token_a -i))
38+
$(eval TOKEN_B := $(shell icp canister status token_b -i))
39+
$(eval ALICE := $(shell icp identity principal --identity icrc2-alice))
40+
$(eval BOB := $(shell icp identity principal --identity icrc2-bob))
41+
$(eval SWAP := $(shell icp canister status backend -i))
42+
43+
@echo "=== Test 1: balances returns empty lists initially ==="
44+
@result=$$(icp canister call --query backend balances '()') && \
45+
echo "$$result" && \
46+
echo "$$result" | grep -q 'vec {}' && \
47+
echo "PASS" || (echo "FAIL" && exit 1)
48+
49+
@echo "=== Test 2: swap with no deposits returns InsufficientBalance ==="
50+
@result=$$(icp canister call backend swap "(record { \
51+
user_a = principal \"$(ALICE)\"; \
52+
user_b = principal \"$(BOB)\" \
53+
})") && \
54+
echo "$$result" && \
55+
echo "$$result" | grep -q 'InsufficientBalance' && \
56+
echo "PASS" || (echo "FAIL" && exit 1)
57+
58+
@echo "=== Test 3: icrc2-alice approves and deposits token_a ==="
59+
@# Approve amount = deposit (100_000_000) + transfer_fee (10_000) = 100_010_000.
60+
@# The ledger charges a fee on icrc2_transfer_from, so the approval must cover it.
61+
@icp canister call --identity icrc2-alice token_a icrc2_approve "(record { \
62+
amount = 100_010_000 : nat; \
63+
spender = record { owner = principal \"$(SWAP)\"; subaccount = null }; \
64+
expires_at = null; expected_allowance = null; fee = null; \
65+
from_subaccount = null; memo = null; created_at_time = null \
66+
})"
67+
@result=$$(icp canister call --identity icrc2-alice backend deposit "(record { \
68+
token = principal \"$(TOKEN_A)\"; \
69+
from = record { owner = principal \"$(ALICE)\"; subaccount = null }; \
70+
amount = 100_000_000 : nat; \
71+
spender_subaccount = null; fee = null; memo = null; created_at_time = null \
72+
})") && \
73+
echo "$$result" && \
74+
echo "$$result" | grep -q 'ok' && \
75+
echo "PASS" || (echo "FAIL" && exit 1)
76+
77+
@echo "=== Test 4: icrc2-bob approves and deposits token_b ==="
78+
@icp canister call --identity icrc2-bob token_b icrc2_approve "(record { \
79+
amount = 100_010_000 : nat; \
80+
spender = record { owner = principal \"$(SWAP)\"; subaccount = null }; \
81+
expires_at = null; expected_allowance = null; fee = null; \
82+
from_subaccount = null; memo = null; created_at_time = null \
83+
})"
84+
@result=$$(icp canister call --identity icrc2-bob backend deposit "(record { \
85+
token = principal \"$(TOKEN_B)\"; \
86+
from = record { owner = principal \"$(BOB)\"; subaccount = null }; \
87+
amount = 100_000_000 : nat; \
88+
spender_subaccount = null; fee = null; memo = null; created_at_time = null \
89+
})") && \
90+
echo "$$result" && \
91+
echo "$$result" | grep -q 'ok' && \
92+
echo "PASS" || (echo "FAIL" && exit 1)
93+
94+
@echo "=== Test 5: swap alice's token_a for bob's token_b ==="
95+
@result=$$(icp canister call backend swap "(record { \
96+
user_a = principal \"$(ALICE)\"; \
97+
user_b = principal \"$(BOB)\" \
98+
})") && \
99+
echo "$$result" && \
100+
echo "$$result" | grep -q 'ok' && \
101+
echo "PASS" || (echo "FAIL" && exit 1)
102+
103+
@echo "=== Test 6: alice withdraws token_b (received via swap) ==="
104+
@# The backend deducts (amount + transfer_fee) from alice's internal balance.
105+
@# Internal balance = 100_000_000; withdraw 99_990_000 so deduction = 99_990_000 + 10_000 = 100_000_000.
106+
@set -e; \
107+
before=$$(icp canister call --query token_b icrc1_balance_of \
108+
"(record { owner = principal \"$(ALICE)\"; subaccount = null })" | grep -oE '[0-9_]+' | tr -d '_'); \
109+
result=$$(icp canister call --identity icrc2-alice backend withdraw "(record { \
110+
token = principal \"$(TOKEN_B)\"; \
111+
to = record { owner = principal \"$(ALICE)\"; subaccount = null }; \
112+
amount = 99_990_000 : nat; \
113+
fee = null; memo = null; created_at_time = null \
114+
})"); \
115+
echo "$$result"; \
116+
echo "$$result" | grep -q 'ok' || (echo "FAIL" && exit 1); \
117+
after=$$(icp canister call --query token_b icrc1_balance_of \
118+
"(record { owner = principal \"$(ALICE)\"; subaccount = null })" | grep -oE '[0-9_]+' | tr -d '_'); \
119+
delta=$$((after - before)); \
120+
echo "token_b balance: before=$$before after=$$after delta=$$delta"; \
121+
[ "$$delta" -eq 99990000 ] || (echo "FAIL: expected delta 99990000" && exit 1); \
122+
echo "PASS"
123+
124+
@echo "=== Test 7: bob withdraws token_a (received via swap) ==="
125+
@set -e; \
126+
before=$$(icp canister call --query token_a icrc1_balance_of \
127+
"(record { owner = principal \"$(BOB)\"; subaccount = null })" | grep -oE '[0-9_]+' | tr -d '_'); \
128+
result=$$(icp canister call --identity icrc2-bob backend withdraw "(record { \
129+
token = principal \"$(TOKEN_A)\"; \
130+
to = record { owner = principal \"$(BOB)\"; subaccount = null }; \
131+
amount = 99_990_000 : nat; \
132+
fee = null; memo = null; created_at_time = null \
133+
})"); \
134+
echo "$$result"; \
135+
echo "$$result" | grep -q 'ok' || (echo "FAIL" && exit 1); \
136+
after=$$(icp canister call --query token_a icrc1_balance_of \
137+
"(record { owner = principal \"$(BOB)\"; subaccount = null })" | grep -oE '[0-9_]+' | tr -d '_'); \
138+
delta=$$((after - before)); \
139+
echo "token_a balance: before=$$before after=$$after delta=$$delta"; \
140+
[ "$$delta" -eq 99990000 ] || (echo "FAIL: expected delta 99990000" && exit 1); \
141+
echo "PASS"

0 commit comments

Comments
 (0)