feat: write synthetic EVM Transfer log for non-custodial transfers#253
feat: write synthetic EVM Transfer log for non-custodial transfers#253sadiq1971 wants to merge 1 commit into
Conversation
After a successful /api/v2/transfer/execute, write a synthetic EVM block containing a Transfer log so eth_getLogs / the Activity page shows non-custodial (snap-signed) transfers alongside custodial ones. EVM metadata (from/to EVM addr, contract address, amount) is captured into a sync.Map at Prepare time and consumed at Execute time. evmStore is nil when EthRPC is disabled so the path is zero-cost then.
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #253 +/- ##
=======================================
Coverage ? 31.96%
=======================================
Files ? 127
Lines ? 9073
Branches ? 0
=======================================
Hits ? 2900
Misses ? 5921
Partials ? 252
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements synthetic EVM log generation for non-custodial transfers to ensure they appear in activity logs and external indexers. It introduces a CachedTransfer struct to store EVM-specific metadata during the preparation phase and adds logic to the Execute method to persist these logs via a new EvmLogStore interface. Review feedback suggests improving data integrity by failing the preparation phase if amount parsing fails, addressing potential race conditions in nonce management, and adding validation to prevent negative amounts from being processed.
| amount, parseErr := parseTokenAmount(req.Amount, entry.decimals) | ||
| if parseErr != nil { | ||
| amount = new(big.Int) // fall back to zero; log will show 0 amount | ||
| } |
There was a problem hiding this comment.
Falling back to a zero amount when parsing fails is risky as it results in misleading synthetic logs for successful transfers. Since PrepareTransfer has already succeeded, any parsing failure here indicates an inconsistency between the Canton amount format and this local parser. It is better to return an error and fail the Prepare call to ensure data integrity between the Canton and EVM layers.
| amount, parseErr := parseTokenAmount(req.Amount, entry.decimals) | |
| if parseErr != nil { | |
| amount = new(big.Int) // fall back to zero; log will show 0 amount | |
| } | |
| amount, err := parseTokenAmount(req.Amount, entry.decimals) | |
| if err != nil { | |
| return nil, apperrors.BadRequestError(err, "invalid amount format for EVM log") | |
| } |
| // EVM transaction record) for a completed non-custodial transfer. Errors here | ||
| // are non-fatal: the Canton transfer already succeeded. | ||
| func (s *TransferService) writeEvmTransferLog(ctx context.Context, cached *CachedTransfer) error { | ||
| nonce, err := s.evmStore.GetEvmTransactionCount(ctx, cached.SenderEVMAddr) |
There was a problem hiding this comment.
Retrieving the nonce via GetEvmTransactionCount without a lock or a sequence generator in the store is prone to race conditions if multiple transfers for the same sender are executed concurrently. This could lead to duplicate nonces in the synthetic EVM history, causing conflicts when the activity page or external indexers process these logs. Consider using a more robust nonce management strategy if high concurrency is expected.
| func parseTokenAmount(amountStr string, decimals int) (*big.Int, error) { | ||
| parts := strings.SplitN(strings.TrimSpace(amountStr), ".", 2) | ||
| whole := strings.ReplaceAll(parts[0], ",", "") | ||
| frac := "" | ||
| if len(parts) == 2 { | ||
| frac = parts[1] | ||
| } | ||
| if len(frac) > decimals { | ||
| return nil, fmt.Errorf("too many decimal places in %q", amountStr) | ||
| } | ||
| frac += strings.Repeat("0", decimals-len(frac)) | ||
| raw := whole + frac | ||
| result := new(big.Int) | ||
| if _, ok := result.SetString(raw, 10); !ok { | ||
| return nil, fmt.Errorf("invalid amount %q", amountStr) | ||
| } | ||
| return result, nil | ||
| } |
There was a problem hiding this comment.
The parseTokenAmount function does not explicitly check for negative amounts. While the Canton layer likely validates this, big.Int.Bytes() returns the absolute value, meaning a negative amount string would be silently logged as a positive value in the EVM layer. You should validate that the amount is non-negative before processing.
func parseTokenAmount(amountStr string, decimals int) (*big.Int, error) {
amountStr = strings.TrimSpace(amountStr)
if strings.HasPrefix(amountStr, "-") {
return nil, fmt.Errorf("negative amount not allowed: %q", amountStr)
}
parts := strings.SplitN(amountStr, ".", 2)
whole := strings.ReplaceAll(parts[0], ",", "")
frac := ""
if len(parts) == 2 {
frac = parts[1]
}
if len(frac) > decimals {
return nil, fmt.Errorf("too many decimal places in %q", amountStr)
}
frac += strings.Repeat("0", decimals-len(frac))
raw := whole + frac
result := new(big.Int)
if _, ok := result.SetString(raw, 10); !ok {
return nil, fmt.Errorf("invalid amount %q", amountStr)
}
return result, nil
}
sadiq1971
force-pushed
the
sadiq/fix/non-custodial-evm-logs
branch
from
f6635d4 to
a28a1e0
Compare
2 participants
After a successful /api/v2/transfer/execute, the TransferService now opens a synthetic EVM block and persists an ERC-20 Transfer log entry so that eth_getLogs / the Activity page reflects non-custodial (snap-signed) transfers alongside custodial ones.
Key changes: