## Summary Memoless transactions (introduced to work around memo-size limits by registering a memo once and referencing it via a short numeric ID) currently work for gas assets, but their behaviour for non-gas EVM assets (ERC20s) is untested end-to-end and — for direct-to-vault transfers — not supported at all. This issue tracks the work to: 1. **Verify** memoless transactions work for ERC20s sent via the router. 2. **Implement** memoless + regular inbound support for ERC20s sent directly to a vault address (whitelisted tokens only). 3. **Add test coverage** for refund paths on both router and direct-vault inbound failures. 4. **Add test coverage** for vault churns correctly migrating ERC20 balances (independently of which path they arrived through). Scope is limited to **ETH** for the first pass. BSC / AVAX / BASE / POL can follow once the pattern is validated. --- ## Current State ### THORChain (x/thorchain) — decimal-aware memoless logic exists - `x/thorchain/handler_observed_tx_helpers.go:1048` — `ExtractReferenceFromAmount()` already looks up `pool.Decimals` for non-gas assets and normalizes before extracting the reference. - `x/thorchain/handler_observed_tx_helpers.go:187` — `generateReferenceMemoID()` runs on empty-memo inbounds and synthesizes an `r:XXXXX` memo. - `x/thorchain/handler_observed_tx_helpers.go:276` — `fetchMemoFromReference()` resolves the reference to the original memo. So, in principle, memoless should support ERC20s **as long as bifrost surfaces the inbound with the right asset/decimals**. This needs to be verified with dedicated tests. ### Bifrost / EVM observation — two distinct paths **Path A — ERC20 sent via `Router.depositWithExpiry(vault, token, amount, memo, expiry)`:** - `bifrost/pkg/chainclients/shared/evm/smartcontract_log_parser.go:176-206` parses the router's `Deposit` event. The memo is taken directly from the event, the amount runs through `amtConverter` to normalize to 1e8. - Both memoless sub-modes *should* work: - `r:XXXXX` memo passed as a literal string → carried on the `Deposit` event → resolved by `fetchMemoFromReference`. - Empty memo + amount-encoded ref → `generateReferenceMemoID()` extracts ref from the normalized amount using `pool.Decimals`. - **Unverified** whether this actually works for tokens with decimals ≠ 18 (e.g. USDC/USDT at 6). The math around `amtConverter` → normalize to 1e8 → `ExtractReferenceFromAmount` dividing back down needs explicit test coverage. **Path B — ERC20 sent directly to vault (`token.transfer(vault, amount)`):** - **Completely unobserved today.** `smartcontract_log_parser.go:171` has `if !scp.addressValidator(&item.Address, false) { continue }`, which only accepts logs emitted by the router contract. A raw `Transfer` event is emitted by the token contract, so it is filtered out. - After log parsing, if `txInItem.Coins.IsEmpty()`, the tx is dropped at `evm_block_scanner.go:1053-1055`. - There is no handling for ERC20 `Transfer(from, to, amount)` (topic `0xddf252ad...`) anywhere in the EVM scanner. - Consequence: a user who sends USDC directly to the asgard vault address receives no credit — the funds are effectively lost outside of manual recovery. ### Simulation coverage — gaps - `test/simulation/actors/suites/swaps.go:30-41` — `MemolessSwaps()` already iterates EVM non-gas tokens, but has not been verified to actually pass for 6-decimal tokens. - `test/simulation/actors/core/swap_memoless.go:346-484` — `signTokenSwap()` always goes through the router. There is **no direct `token.transfer(vault, amount)` path**. - `test/simulation/actors/core/churn.go:19-136` — churn actor triggers a churn but only asserts vault status transitions, **not balances**. No existing test verifies ERC20 balances migrate during churn. - No existing test sends an ERC20 with an invalid/memoless memo and verifies an ERC20 refund is issued. --- ## Design Decisions Captured here so implementation work doesn't need to re-open the discussion: ### 1. Direct-transfer whitelist Use the existing EVM token whitelist already loaded by the bifrost chain client: - Source of truth: `common/tokenlist/ethtokens/eth_{mainnet,mocknet}_latest.json` - Loader: `common/tokenlist/eth.go:11-19` → `GetETHTokenList()` → dispatched via `common/tokenlist/token_list.go:37-55` - Already held in `bifrost/pkg/chainclients/shared/evm/token_manager.go:35,48` as `tokenWhitelist []tokenlist.ERC20Token` Note: `token_list.go:36` has a comment warning that the JSON list is "not necessarily" identical to the bifrost chain-client whitelist. Implementation should double-check which list the EVM chain client actually enforces at runtime and use the same one — we do not want divergence between deposit acceptance and direct-transfer acceptance. ### 2. Sender attribution for direct transfers Use `Transfer.from` (indexed topic), **not** `tx.From`. Rationale: `Transfer.from` is the address whose balance actually decreased. `tx.From` is correct only when the sender is an EOA calling `token.transfer()` directly. It is wrong for every other common case: multisig wallets, meta-transactions / relayers, and `transferFrom` pulls by approved third parties. Using `tx.From` would silently misdirect refunds in those flows. ### 3. Direct-transfer filter rule Accept an ERC20 `Transfer(from, to, amount)` event as an inbound **iff all of the following are true**: 1. `log.Address` (the token contract) is in the EVM chain client's token whitelist. 2. `to` equals an active or retiring vault address on this chain. 3. `from != address(0)` (mint events — ignored). 4. `from != vault address` (vault-to-self — ignored). 5. `from != router address` (already counted via the router's `Deposit` event). 6. `amount > 0`. If `Transfer.from != tx.From`, log informationally but do not reject. ### 4. Gas accounting Direct transfers do not interact with the router, so there is no router event to attribute gas against. Bifrost still sees `tx.GasUsed * tx.GasPrice` from the receipt — propose attributing that as the inbound gas cost the same way gas-asset direct sends already do. Confirm during implementation. ### 5. Minimum refund value Reuse the existing minimum-value / gas-cost check in `refundTx` (`x/thorchain/helpers.go:27`). Small direct transfers that can't afford their own refund gas are absorbed, same as today. No new threshold. ### 6. Direct transfers during a churn Propose documenting current behaviour (inbound lands on whichever vault address it was sent to; if that is the retiring vault, it gets re-migrated on the next migration cycle) and adding a test that covers the happy path but does not enforce any specific ordering guarantee. No new code required for this — it is a verification-only item. --- ## Work Breakdown ### A. Verify Path A (router memoless for ERC20s) works today - [ ] Unit test in `x/thorchain/handler_observed_tx_helpers_test.go`: exercise `ExtractReferenceFromAmount` with an ERC20 pool at `Decimals = 6` (USDC-style) and at `Decimals = 18`. Assert that `r:XXXXX` extraction is correct across the decimal boundary. - [ ] Unit test in `x/thorchain/handler_observed_txin_test.go`: full memoless resolve flow with an ERC20 asset (both `Decimals = 6` and `Decimals = 18`). - [ ] Simulation: run `MemolessSwaps()` against ETH with at least one 6-decimal token (USDC or USDT) and one 18-decimal token, for both `MemolessTypeRef` and `MemolessTypeAmount`. If any test fails, file a fix as part of this work. ### B. Add refund coverage for Path A memoless failures New simulation actors (model after `test/simulation/actors/features/inactive_vault_refunds.go`) sending ERC20s via `depositWithExpiry()` with: - [ ] Unparseable memo → verify ERC20 refund outbound. - [ ] `r:XXXXX` pointing to a nonexistent reference → verify ERC20 refund outbound. - [ ] `r:XXXXX` pointing to an expired reference → verify ERC20 refund outbound. - [ ] Valid reference whose resolved memo targets an unpoolable asset → verify ERC20 refund outbound. - [ ] Assert that refunded asset is the original ERC20, not swapped to gas. ### C. Implement Path B (direct ERC20 → vault) observation - [ ] `bifrost/pkg/chainclients/shared/evm/smartcontract_log_parser.go`: add a new branch in `GetTxInItem()` handling the ERC20 `Transfer` topic (`0xddf252ad...`). Apply the filter rule from Design Decision #3. Use the whitelist from `token_manager`. Populate `txInItem.Sender` from `Transfer.from`, `txInItem.To` from `Transfer.to`, `txInItem.Coins` via the existing `amtConverter` / `decimalResolver` pipeline so normalization to 1e8 happens the same way as the router path. - [ ] `bifrost/pkg/chainclients/evm/evm_block_scanner.go`: thread the set of vault addresses through to the parser so `to` matching can work. Confirm empty-coins short-circuit at line 1053-1055 no longer drops direct-transfer inbounds. - [ ] Deduplication guard: if a single tx hash surfaces both a router `Deposit` event and a raw `Transfer` to vault (shouldn't happen under normal use, but defensively), do not double-count. Prefer the router event when both are present. - [ ] Unit tests in `bifrost/pkg/chainclients/shared/evm/smartcontract_log_parser_test.go` covering every branch of the filter rule (whitelisted+valid, non-whitelisted ignored, `from=0` ignored, `from=vault` ignored, `from=router` ignored, `to` not a vault ignored, `amount=0` ignored). - [ ] Log-only test: `Transfer.from != tx.From` happy case (e.g. `transferFrom` flow) produces a valid TxIn with `Sender = Transfer.from`. ### D. Simulation coverage for Path B - [ ] New `signDirectTokenSwap()` helper in `test/simulation/actors/core/swap_memoless.go` that builds a raw `token.transfer(vault, tokenAmount)` sim tx (no `approve`, no router). Gate behind a new actor option so existing router coverage is unchanged. - [ ] Extend `test/simulation/actors/suites/swaps.go MemolessSwaps()` to enqueue a direct-path memoless actor per whitelisted ETH ERC20 (amount-encoded only, since `transfer()` has no memo field). - [ ] Add a non-memoless direct-path sibling actor (regular swap with a memo-less amount encoding) so we can distinguish "memoless broken" from "direct path broken" in failure diagnostics. - [ ] Include at least one 6-decimal token and one 18-decimal token in the ETH direct-path coverage. ### E. Refund coverage for Path B - [ ] EOA direct transfer → amount that does not decode to a valid reference → verify refund arrives at the sending EOA (`Transfer.from == tx.From` happy path). - [ ] `transferFrom` flow: contract A pulls tokens from EOA B into the vault → invalid memoless reference → verify refund arrives at **B** (the `Transfer.from`), not A. - [ ] Direct transfer with a valid reference that has expired → verify refund. - [ ] Direct transfer with a valid reference whose target asset is unpoolable → verify refund. - [ ] `Transfer.from == address(0)` (minted directly to vault via a weird token) → verify no TxIn, no refund, no crash. - [ ] `Transfer.from` is a contract address → verify refund is still issued to the contract address (document that we make no guarantees about the contract being able to receive it). ### F. Churn verification (independent of Path B) - [ ] Extend `test/simulation/actors/core/churn.go` with a variant (or add a new actor) that asserts post-churn **balance** correctness, not just vault status transitions. - [ ] Set up pools for at least one 6-decimal and one 18-decimal ERC20 on ETH. Perform swaps so the asgard vault holds non-trivial balances of both ERC20s and the gas asset. Trigger churn. Assert: - After migration completes, the new active vault's `Coins` contains the expected ERC20 balances (allowing for migration gas). - The retiring vault's balances are zeroed. - The on-chain balance at the new vault address matches (`transferAllowance` event observed). ### G. Cross-path churn test (requires Path B landed) - [ ] Perform one swap via router (Path A) and one via direct transfer (Path B) so the vault holds ERC20s that arrived from both sources. Trigger churn. Assert both sets of funds migrate — guards against an accidental tagging/metadata difference between path-A and path-B coins. ### H. Spam protection - [ ] Test: send a **non-whitelisted** ERC20 directly to the vault. Assert bifrost ignores it (no TxIn, no refund, no crash, no log spam at error level). --- ## Key File References **x/thorchain (memoless core logic — already decimal-aware):** - `x/thorchain/handler_observed_tx_helpers.go:187` — `generateReferenceMemoID` - `x/thorchain/handler_observed_tx_helpers.go:276` — `fetchMemoFromReference` resolution point - `x/thorchain/handler_observed_tx_helpers.go:933-994` — `fetchMemoFromReference` impl - `x/thorchain/handler_observed_tx_helpers.go:1048-1116` — `ExtractReferenceFromAmount` with pool decimals - `x/thorchain/helpers.go:27` — `refundTx` **Bifrost (changes required for Path B):** - `bifrost/pkg/chainclients/shared/evm/smartcontract_log_parser.go:160-341` — `GetTxInItem`, add ERC20 `Transfer` branch - `bifrost/pkg/chainclients/shared/evm/smartcontract_log_parser.go:20-26` — event topic constants - `bifrost/pkg/chainclients/evm/evm_block_scanner.go:1053-1055` — empty-coins short-circuit - `bifrost/pkg/chainclients/shared/evm/token_manager.go:35,48` — whitelist storage **Whitelist source:** - `common/tokenlist/ethtokens/eth_{mainnet,mocknet}_latest.json` - `common/tokenlist/eth.go:11-19` - `common/tokenlist/token_list.go:37-55` **Simulation (test additions):** - `test/simulation/actors/core/swap_memoless.go:346-484` — `signTokenSwap` (router path to mirror) - `test/simulation/actors/core/swap_memoless.go:515-591` — existing decimal scaling helpers - `test/simulation/actors/suites/swaps.go:17-47` — `MemolessSwaps()` suite entry point - `test/simulation/actors/features/inactive_vault_refunds.go` — refund actor to model new refund tests after - `test/simulation/actors/core/churn.go:19-136` — churn actor to extend **Migration / churn (expected to work without changes):** - `x/thorchain/manager_network_current.go:495-835` — `migrateFunds` (no asset-type filtering; ERC20s already eligible) - `x/thorchain/handler_observed_tx_helpers.go:216` — `vault.AddFunds` (shared path for router and direct transfer) - `x/thorchain/types/type_vault.go:226-246` — `AddFunds` --- ## Out of Scope - BSC / AVAX / BASE / POL direct-transfer support (follow-up issues once ETH pattern is validated). - Aggregator flows (`transferOutAndCall`) for memoless — not affected by this work. - UTXO-style chain memoless (already covered by existing unit tests). - Changing the memoless reference length, TTL, or max-use constants. - Product work on the whitelist itself (adding/removing tokens). ## Acceptance Criteria - All items in sections A–H are implemented and tests pass in CI. - `make test-regression` passes. - A user sending an unpooled USDC amount directly to an asgard vault address (whitelisted, amount-encoded reference) successfully triggers a swap. - A user sending a whitelisted ERC20 directly to an asgard vault with an invalid reference receives a refund at the `Transfer.from` address. - After a churn, ERC20 balances (from both router and direct-transfer sources) are correctly migrated to the new active vault. - Non-whitelisted tokens sent directly to the vault are silently ignored with no side effects.