Feature/blk 129 precompile for the delegate state in parachainstaking#96
Open
sfffaaa wants to merge 23 commits into
Open
Feature/blk 129 precompile for the delegate state in parachainstaking#96sfffaaa wants to merge 23 commits into
sfffaaa wants to merge 23 commits into
Conversation
added 22 commits
August 14, 2025 22:13
- Create test suite with 11 delegators distributed across 2 collators (7+5) - Implement 2 multi-delegators for complex delegation patterns - Test pagination with offset/limit parameters (max 512 limit) - Validate edge cases: limit=0 rejection, non-existent delegators - Verify EVM results match Substrate state - Test gas consumption for bulk queries - Include extended ABI for getDelegatorState functions
- Move collator setup to class level (4 collators guaranteed) - Create dedicated test delegator with 4 delegations in class setup - Remove incorrect 1-parameter getDelegatorState function from ABI - Simplify test_get_delegator_state_large_delegation_set - Fix funding amounts to use proper minimum delegation values - Update all tests to use class-level collator_list and test_delegator - Remove duplicate _setup_multiple_delegators_and_collators method
After removing the incorrect 1-parameter version from ABI, update test to use the 3-parameter version with offset=0 and limit=10
The test was redundant with existing class setup which already creates 11 delegators (7 to collator1, 4 to collator2) and is covered by other comprehensive tests.
- Send all 11 join delegations in parallel instead of sequentially - Add force_new_round between joins and multi-delegations to avoid DelegationsPerRoundExceeded - Implement retry logic for failed transactions - Add detailed progress logging for debugging - Significant performance improvement: ~60-80% faster setup time
- Break down _setup_class_delegators from 265 lines to 13 lines - Extract infrastructure setup, main delegators, and pagination delegator setup - Add async processing pattern to eliminate code duplication - Improve error handling and transaction sequencing
- Update ABI to accept address type instead of bytes32 for getDelegatorState input - Add convertEthToSubstrateAccount function to ABI for ETH to substrate conversion - Fix all test methods to use ETH addresses instead of bytes(32) for getDelegatorState calls - Update test assertions to properly compare substrate bytes32 responses with converted addresses - Improve async delegation setup with better retry logic and transaction receipt handling - Add proper checksum address validation for fake delegator tests These changes align with the precompile updates that now use ETH addresses as input while returning substrate bytes32 addresses in responses.
- Remove all collator count checks - convert to assertions ensuring setup guarantees - Remove receipt success checks - convert to proper assertions with error messages - Remove EVM receipt status checks - convert to assertEqual for cleaner validation - Simplify conditional validation logic: * Extract _validate_exact_delegator_state() helper to eliminate multi/single delegator if/else * Simplify multi-delegator counting using inline conditional expressions * Replace known delegator if statements with short-circuit evaluation - Optimize async delegation processing: * Reduce wait times from 3x to 1x BLOCK_GENERATE_TIME per batch * Implement two-pass receipt checking (immediate + retry) * Remove redundant sleeps after force_new_round operations These changes eliminate 15+ conditional if statements, making tests more deterministic and reducing execution time while maintaining full validation coverage.
Performance Optimizations: - Reduce collator requirement from 4 to 2 (eliminates 2 expensive collator creations) - Update pagination delegator to use 2 delegations instead of 4 - Modify large delegation test to work with 2 collators instead of 4 Caching Infrastructure: - Cache SubstrateInterface, Web3, and contract instances at class level - Cache collator list to avoid repeated getCollatorList() calls - Reuse cached connections in setUp() and instance methods - Eliminate redundant connection creation across test methods Expected Performance Impact: - Saves ~50% of collator setup time (2 fewer collator creations) - Eliminates multiple SubstrateInterface and Web3 instance creations - Reduces blockchain RPC calls through collator list caching - Maintains full test coverage with faster execution These optimizations should significantly reduce the 19+ minute test execution time while preserving all test functionality and validation coverage.
… class methods - assertEqual is an instance method, not available in @classmethod - Use simple assert statements for class-level validation - Maintains same validation logic with proper class method compatibility
- Remove conditional hasattr checks in favor of direct access - _get_collator_list() now directly returns class-level collator_list - _fund_users() uses keypairs from setUp() without conditional logic - More explicit and predictable test behavior - Easier to debug and maintain In testing, explicit control is better than magic fallbacks.
- Add descriptive messages to assertions that were missing them - Improve exception messages to be more specific about the failure cause - Use more appropriate exception types (RuntimeError, ValueError) - Make test failures easier to diagnose and debug Better error messages help quickly identify issues when tests fail.
Major refactoring to improve code quality, maintainability and performance: Method Decomposition: - Split _process_delegations_async (146 lines) into 6 focused methods - Split _setup_main_delegators (67 lines) into 5 helper methods - Each method now follows single responsibility principle Code Quality Improvements: - Added 5 assertion helper methods to reduce code duplication - Replaced magic numbers with descriptive constants - Standardized exception handling patterns (removed bare except) - Improved error messages and logging context Performance Results: - Original: 19+ minutes (1144.90s) - After optimizations: 12 minutes 16 seconds (736.05s) - 36% reduction in execution time - All 13 tests passing successfully Maintainability: - No methods exceed 50 lines - Self-documenting method and constant names - Reusable validation patterns - Easier to debug and extend
- Added @requires_collators decorator to eliminate repeated assertions - Added comprehensive class docstring explaining test setup - Added section comments to group related tests - Improved code organization without changing functionality All tests remain passing with same performance (~12 minutes)
- Split complex _setup_infrastructure into 3 focused methods: - _get_min_delegation(): Get chain constants - _get_or_create_collator_list(): Cache management - _ensure_minimum_collators(): Collator creation logic - _setup_infrastructure now just orchestrates these simple methods - Each method has single responsibility - Easier to understand and maintain
- Fixed unused imports (removed TestUtils, KP_COLLATOR, get_block_hash) - Fixed line length issues (E501) - split long lines to stay under 120 chars - Fixed whitespace issues (removed trailing whitespace, extra blank lines) - Fixed indentation issues (E128) - properly aligned continuation lines - Fixed f-string without placeholders - Used autopep8 for consistent formatting All flake8 checks now pass with max-line-length=120
- Moved repeated '0x0000000000000000000000000000000000000000' to ZERO_ADDRESS constant - Used in 9 places across multiple test methods - Reduces code duplication and improves maintainability - Single source of truth for the zero address used to query all delegators
- Create all collator keypairs upfront instead of in loop - Batch fund all new collators in single transaction (was N transactions) - Still join collators individually (protocol requirement) - Update cache once at end instead of per iteration - Early return if already have enough collators Performance improvement: Reduces transaction count from 2N+1 to N+2 for N new collators
- Replace magic number 11 with descriptive constant - Improves code maintainability for delegator count changes
- Add fund_test_accounts() utility in tools/peaq_eth_utils.py - Support both force_set_balance and transfer_keep_alive methods - Replace duplicate _fund_users implementations across 4 test files - Reduce code duplication and improve maintainability - Remove unused imports (ExtrinsicBatch, KP_GLOBAL_SUDO) Files updated: - tests/test_get_delegator_state.py: 35 lines -> 14 lines - tests/bridge_parachain_staking_test.py: 38 lines -> 12 lines - tests/bridge_multiple_collator_test.py: 29 lines -> 9 lines - tests/bridge_asset_factory_test.py: 19 lines -> 8 lines
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new precompile function getDelegatorState for querying delegator information in parachain staking, along with helper functions and comprehensive test coverage.
- Adds
getDelegatorStateprecompile functionality with pagination support - Introduces
fund_test_accountsutility function for batch funding operations - Includes extensive test suite covering basic functionality, pagination, edge cases, and performance testing
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/peaq_eth_utils.py | Adds fund_test_accounts utility function for batch funding multiple accounts |
| tests/test_get_delegator_state.py | Complete test suite for new getDelegatorState precompile functionality |
| tests/bridge_parachain_staking_test.py | Refactors funding logic to use new fund_test_accounts utility |
| tests/bridge_multiple_collator_test.py | Refactors funding logic to use new fund_test_accounts utility |
| tests/bridge_asset_factory_test.py | Refactors funding logic to use new fund_test_accounts utility |
| ETH/parachain-staking/abi | Adds ABI definitions for new precompile functions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
sfffaaa
force-pushed
the
feature/blk-129-precompile-for-the-delegate-state-in-parachainstaking
branch
from
a8c762d to
adc2c05
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
sfffaaa
force-pushed
the
feature/blk-129-precompile-for-the-delegate-state-in-parachainstaking
branch
from
adc2c05 to
b634ac7
Compare
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
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.
No description provided.