-
Notifications
You must be signed in to change notification settings - Fork 18
Refactor: Consolidate EVM chain code and reduce duplication #2725
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
+632
−615
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…y fallback - Replace switch statements with Map-based lookups in BlockchainRegistryService - Consolidate EVM chain handling in blockchain.util.ts and CryptoService using EvmBlockchains array - Move duplicate onModuleInit() logic from 7 register strategies to AlchemyStrategy base class - Add STANDARD_EVM_WALLET_PRIVATE_KEY fallback in config.ts to reduce .env duplication Code reduction: ~305 lines
- Add AssetService.getNativeCoin() to eliminate 8 duplicate get*Coin() calls - Consolidate 8 Payout Prepare Strategies to use getNativeCoin() - Create PayInEvmFactory with proxy pattern for EVM chain services - Reduce PayIn service boilerplate from ~10 lines to chain-specific delegation Code reduction: ~230 lines
- Create IPayInEvmService interface for type safety - Update EvmStrategy base classes to accept interface instead of concrete class - Convert PayInEvmService from abstract to concrete class - Add getCurrentBlockNumber() to PayInEvmProxyService - Fix BlockchainRegistryService Map type inference by explicitly typing array
- Change EVM_SERVICE_MAP to Partial<Record<>> instead of Record<> - Change NATIVE_COIN_MAP to Partial<Record<>> instead of Record<> - Remove all explicit undefined entries (not needed with Partial) - Reduces code by 38 lines while improving type safety
- Remove getWalletPrivateKey() fallback function - Use STANDARD_EVM_WALLET_PRIVATE_KEY directly for 8 standard EVM chains (Ethereum, Sepolia, BSC, Optimism, Arbitrum, Polygon, Base, Gnosis) - Citrea keeps separate key (non-standard chain) - Consolidate .env.example from 8 separate private keys to 1 - Reduces config.ts by 13 lines - Reduces .env.example by 8 lines Breaking Change: Requires STANDARD_EVM_WALLET_PRIVATE_KEY in .env
- Replace individual chain key generation with single standard key - Generate STANDARD_EVM_WALLET_PRIVATE_KEY for 8 standard EVM chains - Generate separate key only for CITREA_TESTNET (non-standard) - Reduces generated keys from 9 to 2
Changes: - Created PayoutEvmFactory for dynamic service instantiation - Created IPayoutEvmService interface for type safety - Created PayoutEvmProxyService base class for delegation - Updated all 9 EVM payout services to use proxy pattern - Made PayoutEvmService concrete (removed abstract) - Updated EvmStrategy to accept IPayoutEvmService interface Benefits: - Eliminates redundant blockchain service injections - Centralizes service creation logic - Maintains backward compatibility with existing strategies - Cleaner dependency injection structure
Changes: - Created GenericAlchemyStrategy with parametrized blockchain - Reduced 7 strategy files from ~25 lines to ~10 lines each - Centralized wallet address mapping in WALLET_ADDRESS_MAP - Extracted common logic to base generic class Benefits: - Eliminated ~100 lines of duplicated code - Single source of truth for Alchemy strategy logic - Easier to add new EVM chains - Maintained Gnosis custom implementation for special asset mapping
The wallet address map was being initialized at module load time, causing test failures when Config was not yet initialized. Moved the map into a method for lazy initialization.
Critical fixes: - Restored GnosisStrategy custom asset mapping (ETH -> xDAI) - Aligned PayoutEvmService with PayInEvmService structure - Use private field (#client) and store service reference - Added documentation for Gnosis special handling Note: Only 7 Alchemy strategies consolidated, Gnosis kept separate
d06a5a8 to
b4b852e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR consolidates duplicate code across standard EVM chains (Ethereum, Sepolia, BSC, Optimism, Arbitrum, Polygon, Base, Gnosis) and introduces architectural improvements using factory and proxy patterns.
Impact:
Changes
1. Environment Configuration Consolidation
Problem: All EVM chains used identical private keys but required separate env variables.
Solution: Introduced fallback pattern for wallet private keys:
Benefit: Can use single
STANDARD_EVM_WALLET_PRIVATE_KEYinstead of 9 separate variables (while still supporting chain-specific overrides for non-standard chains like Citrea).Files:
src/config/config.ts2. Asset Service Native Coin Consolidation
Before: 8 separate
get*Coin()methods (getEthCoin, getBnbCoin, etc.)After: Single
getNativeCoin(blockchain)method with mapping:Benefit:
Files:
src/shared/models/asset/asset.service.ts,src/subdomains/supporting/payout/strategies/prepare/impl/*.strategy.ts3. PayIn Service Consolidation (Factory + Proxy Pattern)
Before: 9 nearly identical EVM PayIn services (~10 lines each, total ~90 lines)
After: Factory + Proxy pattern
New Architecture:
PayInEvmFactory: Manages service instantiation with lazy-loaded singleton patternPayInEvmProxyService: Abstract base class that delegates to factoryIPayInEvmService: Interface for type-safe dependency injectionBenefit:
Files:
src/subdomains/supporting/payin/services/payin-evm.factory.ts(NEW)src/subdomains/supporting/payin/services/base/payin-evm-proxy.service.ts(NEW)src/subdomains/supporting/payin/services/base/payin-evm.interface.ts(NEW)src/subdomains/supporting/payin/services/payin-*.service.ts(9 files simplified)src/subdomains/supporting/payin/payin.module.ts4. PayIn Strategy Webhook Consolidation
Before: 7 EVM strategies duplicated
onModuleInit()with identical webhook setup (~17 lines each)After: Moved to
AlchemyStrategybase classBenefit: Eliminated ~119 lines of duplicate webhook initialization code
Files:
src/subdomains/supporting/payin/strategies/register/impl/base/alchemy.strategy.tssrc/subdomains/supporting/payin/strategies/register/impl/*.strategy.ts(7 files)5. Blockchain Registry Service Refactoring
Before: Large switch statements for service/client lookup (~84 lines)
After: Map-based lookups with explicit type safety
Benefit:
Files:
src/integration/blockchain/shared/services/blockchain-registry.service.ts6. Crypto Service Switch Statement Consolidation
Before: Large switch statements checking every EVM chain individually
After: Use
EvmBlockchainsarray with.includes()checkBenefit: More maintainable, automatically includes new standard EVM chains
Files:
src/integration/blockchain/shared/services/crypto.service.ts7. Blockchain Util Consolidation
Before: Switch statements for asset/address paths
After: Array-based checks with
EvmBlockchains.includes()Benefit: Cleaner, more maintainable
Files:
src/integration/blockchain/shared/util/blockchain.util.tsType Safety Improvements
Partial<Record<Blockchain, T>>instead ofRecord<Blockchain, T | undefined>Breaking Changes
None. All changes are internal refactorings. Public APIs remain unchanged.
Test Plan
npm run buildnpm run testnpm run lintPerformance
Migration Guide
For Developers
No code changes required. All refactorings are internal.
For DevOps
Optional: Can consolidate 9 EVM wallet private key env variables into single
STANDARD_EVM_WALLET_PRIVATE_KEYvariable (but chain-specific variables still work for backward compatibility).Example: