- Added comprehensive bounds checking to prevent buffer overruns in multicall parsing - Implemented graduated validation system (Strict/Moderate/Permissive) to reduce false positives - Added LRU caching system for address validation with 10-minute TTL - Enhanced ABI decoder with missing Universal Router and Arbitrum-specific DEX signatures - Fixed duplicate function declarations and import conflicts across multiple files - Added error recovery mechanisms with multiple fallback strategies - Updated tests to handle new validation behavior for suspicious addresses - Fixed parser test expectations for improved validation system - Applied gofmt formatting fixes to ensure code style compliance - Fixed mutex copying issues in monitoring package by introducing MetricsSnapshot - Resolved critical security vulnerabilities in heuristic address extraction - Progress: Updated TODO audit from 10% to 35% complete 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
523 lines
17 KiB
Markdown
523 lines
17 KiB
Markdown
# LOW-002: Testing Infrastructure - Detailed Fix Plan
|
|
|
|
**Issue ID:** LOW-002
|
|
**Category:** Quality
|
|
**Priority:** Low
|
|
**Status:** Not Started
|
|
**Generated:** October 9, 2025
|
|
**Estimate:** 8-10 hours
|
|
|
|
## Overview
|
|
This plan expands the testing infrastructure to include comprehensive fuzzing tests, property-based testing for mathematical operations, an integration security test suite, and performance regression tests for security features. The goal is to improve test coverage and catch potential issues before they reach production.
|
|
|
|
## Current Implementation Issues
|
|
- Limited fuzzing test coverage for critical components
|
|
- Lack of property-based testing for mathematical operations
|
|
- Missing integration security test suite
|
|
- No performance regression tests for security features
|
|
|
|
## Implementation Tasks
|
|
|
|
### 1. Expand Fuzzing Test Coverage for All Critical Components
|
|
**Task ID:** LOW-002.1
|
|
**Time Estimate:** 2.5 hours
|
|
**Dependencies:** None
|
|
|
|
Implement comprehensive fuzzing tests for critical system components:
|
|
- Identify critical functions that process untrusted input
|
|
- Create fuzzing functions using Go's new fuzzing framework
|
|
- Focus on parsing, validation, and calculation functions
|
|
- Set up fuzzing in CI pipeline with appropriate timeout values
|
|
|
|
```go
|
|
// Example fuzzing test for transaction parsing
|
|
func FuzzParseTransaction(f *testing.F) {
|
|
// Add interesting seeds
|
|
f.Add([]byte{}) // Empty input
|
|
f.Add([]byte{0x01, 0x02, 0x03})
|
|
f.Add([]byte{0xf8, 0x6c, 0x80, 0x85}) // Potential transaction header
|
|
|
|
f.Fuzz(func(t *testing.T, data []byte) {
|
|
// Test should not panic with any input
|
|
_, err := ParseTransaction(data)
|
|
if err != nil {
|
|
// Log errors only if they represent new bugs, not expected validation failures
|
|
if !isExpectedError(err) {
|
|
t.Errorf("Unexpected error: %v", err)
|
|
}
|
|
}
|
|
})
|
|
}
|
|
|
|
// Example fuzzing for ABI decoding
|
|
func FuzzABIDecode(f *testing.F) {
|
|
f.Add([]byte{}, "uint256")
|
|
f.Add([]byte{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}, "uint256")
|
|
|
|
f.Fuzz(func(t *testing.T, data []byte, typeName string) {
|
|
// Test ABI decoding with various inputs
|
|
abiType, err := NewType(typeName, "", nil)
|
|
if err != nil {
|
|
return // Skip invalid type names
|
|
}
|
|
|
|
_, err = Unpack(abiType, data)
|
|
if err != nil {
|
|
// Log errors only if they represent invalid behavior, not validation failures
|
|
if !isExpectedValidationError(err) {
|
|
t.Errorf("Unexpected error in ABI decoding: %v", err)
|
|
}
|
|
}
|
|
})
|
|
}
|
|
|
|
// Helper function to determine if an error is expected
|
|
func isExpectedError(err error) bool {
|
|
// Define which errors are expected during fuzzing
|
|
expected := []string{
|
|
"invalid input",
|
|
"insufficient data",
|
|
"validation failed",
|
|
}
|
|
|
|
errStr := err.Error()
|
|
for _, exp := range expected {
|
|
if strings.Contains(strings.ToLower(errStr), exp) {
|
|
return true
|
|
}
|
|
}
|
|
return false
|
|
}
|
|
```
|
|
|
|
### 2. Add Property-Based Testing for Mathematical Operations
|
|
**Task ID:** LOW-002.2
|
|
**Time Estimate:** 2 hours
|
|
**Dependencies:** None
|
|
|
|
Implement property-based testing for mathematical operations in the codebase:
|
|
- Test mathematical properties like commutativity, associativity
|
|
- Verify precision and accuracy of calculations
|
|
- Test edge cases and boundary conditions
|
|
- Use tools like `gopter` for property-based testing
|
|
|
|
```go
|
|
// Example property-based test for mathematical operations (using gopter-like approach)
|
|
func TestMathProperties(t *testing.T) {
|
|
// Test property: a + b = b + a (commutativity)
|
|
prop := proptest.ForAll(
|
|
func(nums [2]*big.Int) bool {
|
|
// Test with uint256 values to stay within range
|
|
a := new(uint256.Int).SetBytes(nums[0].Bytes())
|
|
b := new(uint256.Int).SetBytes(nums[1].Bytes())
|
|
|
|
sumAB := new(uint256.Int).Add(a, b)
|
|
sumBA := new(uint256.Int).Add(b, a)
|
|
|
|
return sumAB.Eq(sumBA)
|
|
},
|
|
proptest.GenSlice(proptest.GenBigNat),
|
|
)
|
|
|
|
if err := prop.Testing(t); err != nil {
|
|
t.Errorf("Commutativity property failed: %v", err)
|
|
}
|
|
}
|
|
|
|
// Example for multiplicative identity
|
|
func TestMultiplicativeIdentity(t *testing.T) {
|
|
prop := proptest.ForAll(
|
|
func(num *big.Int) bool {
|
|
a := new(uint256.Int).SetBytes(num.Bytes())
|
|
one := uint256.NewInt(1)
|
|
result := new(uint256.Int).Mul(a, one)
|
|
|
|
return result.Eq(a)
|
|
},
|
|
proptest.GenBigNat,
|
|
)
|
|
|
|
if err := prop.Testing(t); err != nil {
|
|
t.Errorf("Multiplicative identity property failed: %v", err)
|
|
}
|
|
}
|
|
|
|
// Property test for arbitrage calculations
|
|
func TestArbitrageCalculationProperties(t *testing.T) {
|
|
prop := proptest.ForAll(
|
|
func(amount *big.Int, rate *big.Float) bool {
|
|
// Test that arbitrage calculation is consistent
|
|
// Convert to appropriate types
|
|
inputAmount := new(uint256.Int).SetBytes(amount.Bytes())
|
|
|
|
// Ensure rate is in reasonable range
|
|
if rate.Cmp(big.NewFloat(0.000001)) < 0 || rate.Cmp(big.NewFloat(1000000)) > 0 {
|
|
return true // Skip unreasonable rates
|
|
}
|
|
|
|
// Perform arbitrage calculation
|
|
output, err := CalculateArbitrageReturn(inputAmount, rate)
|
|
if err != nil {
|
|
return true // Expected error for invalid inputs
|
|
}
|
|
|
|
// Property: output should be positive when input and rate are positive
|
|
return output.Sign() >= 0
|
|
},
|
|
proptest.GenBigNat,
|
|
proptest.GenBigFloat,
|
|
)
|
|
|
|
if err := prop.Testing(t); err != nil {
|
|
t.Errorf("Arbitrage calculation property failed: %v", err)
|
|
}
|
|
}
|
|
```
|
|
|
|
### 3. Implement Integration Security Test Suite
|
|
**Task ID:** LOW-002.3
|
|
**Time Estimate:** 2 hours
|
|
**Dependencies:** LOW-002.1, LOW-002.2
|
|
|
|
Create a comprehensive integration security test suite:
|
|
- Test security controls in integrated system components
|
|
- Simulate security attack scenarios
|
|
- Test authentication and authorization across components
|
|
- Verify that security measures work together properly
|
|
|
|
```go
|
|
// Example integration security test suite
|
|
func TestIntegrationSecuritySuite(t *testing.T) {
|
|
// Setup test environment with mocked dependencies
|
|
testEnv := setupSecureTestEnvironment(t)
|
|
defer testEnv.Teardown()
|
|
|
|
tests := []struct {
|
|
name string
|
|
setup func(*TestEnvironment)
|
|
runTest func(*testing.T, *TestEnvironment)
|
|
expectError bool
|
|
}{
|
|
{
|
|
name: "transaction_signing_with_invalid_key",
|
|
setup: func(env *TestEnvironment) {
|
|
env.MockKeyManager.AddInvalidKey("malicious_key_id")
|
|
},
|
|
runTest: func(t *testing.T, env *TestEnvironment) {
|
|
_, err := env.TransactionSigner.SignWithKey("malicious_key_id", env.TestTransaction)
|
|
assert.Error(t, err)
|
|
},
|
|
expectError: true,
|
|
},
|
|
{
|
|
name: "double_sign_prevention",
|
|
setup: func(env *TestEnvironment) {
|
|
env.TransactionStore.Clear()
|
|
},
|
|
runTest: func(t *testing.T, env *TestEnvironment) {
|
|
// First signing should succeed
|
|
_, err1 := env.TransactionSigner.SignWithKey("test_key", env.TestTransaction)
|
|
require.NoError(t, err1)
|
|
|
|
// Second signing with same nonce should fail
|
|
_, err2 := env.TransactionSigner.SignWithKey("test_key", env.TestTransaction)
|
|
assert.Error(t, err2)
|
|
},
|
|
expectError: true,
|
|
},
|
|
{
|
|
name: "rate_limiting_integration",
|
|
setup: func(env *TestEnvironment) {
|
|
env.RateLimiter.SetLimit("test_user", 1, time.Second)
|
|
},
|
|
runTest: func(t *testing.T, env *TestEnvironment) {
|
|
// First request should succeed
|
|
success1 := env.RateLimiter.Allow("test_user")
|
|
assert.True(t, success1)
|
|
|
|
// Second request should fail
|
|
success2 := env.RateLimiter.Allow("test_user")
|
|
assert.False(t, success2)
|
|
},
|
|
expectError: false,
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
tt.setup(testEnv)
|
|
tt.runTest(t, testEnv)
|
|
})
|
|
}
|
|
}
|
|
|
|
// Security scenario testing
|
|
func TestSecurityScenarios(t *testing.T) {
|
|
testEnv := setupSecureTestEnvironment(t)
|
|
defer testEnv.Teardown()
|
|
|
|
scenarios := []struct {
|
|
name string
|
|
description string
|
|
execute func() error
|
|
}{
|
|
{
|
|
name: "replay_attack_prevention",
|
|
description: "Verify transactions cannot be replayed",
|
|
execute: func() error {
|
|
// Create and sign a transaction
|
|
tx := testEnv.CreateTestTransaction()
|
|
signedTx, err := testEnv.Signer.SignTx(tx, testEnv.TestKey)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
|
|
// Submit transaction twice - second should fail
|
|
err1 := testEnv.SubmitTransaction(signedTx)
|
|
if err1 != nil {
|
|
return fmt.Errorf("first transaction failed: %w", err1)
|
|
}
|
|
|
|
err2 := testEnv.SubmitTransaction(signedTx) // Replay attempt
|
|
if err2 == nil {
|
|
return fmt.Errorf("replay attack succeeded - second transaction was accepted")
|
|
}
|
|
|
|
return nil
|
|
},
|
|
},
|
|
{
|
|
name: "malicious_contract_interaction",
|
|
description: "Verify protection against malicious contract calls",
|
|
execute: func() error {
|
|
// Create transaction with potentially malicious data
|
|
maliciousData := createMaliciousContractCall()
|
|
|
|
// Attempt to process - should be rejected by validation
|
|
err := testEnv.ProcessContractCall(maliciousData)
|
|
if err == nil {
|
|
return fmt.Errorf("malicious contract call was not rejected")
|
|
}
|
|
|
|
return nil
|
|
},
|
|
},
|
|
}
|
|
|
|
for _, scenario := range scenarios {
|
|
t.Run(scenario.name, func(t *testing.T) {
|
|
if err := scenario.execute(); err != nil {
|
|
t.Errorf("%s failed: %v", scenario.description, err)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
```
|
|
|
|
### 4. Create Performance Regression Tests for Security Features
|
|
**Task ID:** LOW-002.4
|
|
**Time Estimate:** 1.5 hours
|
|
**Dependencies:** LOW-002.1, LOW-002.2, LOW-002.3
|
|
|
|
Develop performance regression tests for security features:
|
|
- Benchmark security-related functions
|
|
- Track performance of validation and verification operations
|
|
- Establish performance baselines and alert on regressions
|
|
- Test performance under load for security operations
|
|
|
|
```go
|
|
// Benchmark security-critical functions
|
|
func BenchmarkTransactionValidation(b *testing.B) {
|
|
tx := createTestTransaction()
|
|
validator := NewTransactionValidator()
|
|
|
|
b.ResetTimer()
|
|
for i := 0; i < b.N; i++ {
|
|
_ = validator.Validate(tx)
|
|
}
|
|
}
|
|
|
|
func BenchmarkSignatureVerification(b *testing.B) {
|
|
tx := createTestTransaction()
|
|
signer, _ := NewTransactionSigner(testPrivateKey)
|
|
signedTx, _ := signer.SignTx(tx, testPrivateKey)
|
|
|
|
b.ResetTimer()
|
|
for i := 0; i < b.N; i++ {
|
|
valid, _ := VerifySignature(signedTx)
|
|
if !valid {
|
|
b.Fatal("Signature verification failed unexpectedly")
|
|
}
|
|
}
|
|
}
|
|
|
|
// Performance regression test with thresholds
|
|
func TestSecurityPerformanceRegression(t *testing.T) {
|
|
// Load baseline performance data
|
|
baselineData := loadPerformanceBaseline()
|
|
|
|
tests := []struct {
|
|
name string
|
|
benchmark func() time.Duration
|
|
threshold time.Duration
|
|
}{
|
|
{
|
|
name: "transaction_validation",
|
|
benchmark: func() time.Duration {
|
|
start := time.Now()
|
|
tx := createTestTransaction()
|
|
validator := NewTransactionValidator()
|
|
_ = validator.Validate(tx)
|
|
return time.Since(start)
|
|
},
|
|
threshold: baselineData.TransactionValidation + (10 * time.Millisecond), // 10ms tolerance
|
|
},
|
|
{
|
|
name: "signature_verification",
|
|
benchmark: func() time.Duration {
|
|
start := time.Now()
|
|
tx := createTestTransaction()
|
|
signer, _ := NewTransactionSigner(testPrivateKey)
|
|
signedTx, _ := signer.SignTx(tx, testPrivateKey)
|
|
_ = VerifySignature(signedTx)
|
|
return time.Since(start)
|
|
},
|
|
threshold: baselineData.SignatureVerification + (5 * time.Millisecond), // 5ms tolerance
|
|
},
|
|
{
|
|
name: "input_validation",
|
|
benchmark: func() time.Duration {
|
|
start := time.Now()
|
|
data := make([]byte, 1024)
|
|
_ = validateInputData(data)
|
|
return time.Since(start)
|
|
},
|
|
threshold: baselineData.InputValidation + (2 * time.Millisecond), // 2ms tolerance
|
|
},
|
|
}
|
|
|
|
for _, tt := range tests {
|
|
t.Run(tt.name, func(t *testing.T) {
|
|
duration := tt.benchmark()
|
|
|
|
if duration > tt.threshold {
|
|
t.Errorf("%s performance regressed: took %v, threshold %v",
|
|
tt.name, duration, tt.threshold)
|
|
}
|
|
|
|
// Log performance for monitoring
|
|
t.Logf("%s took %v", tt.name, duration)
|
|
})
|
|
}
|
|
}
|
|
|
|
// Load test for security features
|
|
func TestSecurityUnderLoad(t *testing.T) {
|
|
if testing.Short() {
|
|
t.Skip("skipping load test in short mode")
|
|
}
|
|
|
|
numWorkers := 10
|
|
numOperations := 1000
|
|
var wg sync.WaitGroup
|
|
|
|
// Start workers to perform security-sensitive operations
|
|
for i := 0; i < numWorkers; i++ {
|
|
wg.Add(1)
|
|
go func(workerID int) {
|
|
defer wg.Done()
|
|
|
|
for j := 0; j < numOperations; j++ {
|
|
// Perform some security-sensitive operation
|
|
tx := createTestTransaction()
|
|
validator := NewTransactionValidator()
|
|
|
|
err := validator.Validate(tx)
|
|
if err != nil {
|
|
t.Errorf("Worker %d, operation %d failed validation: %v", workerID, j, err)
|
|
}
|
|
}
|
|
}(i)
|
|
}
|
|
|
|
wg.Wait()
|
|
|
|
// Verify system performance under load
|
|
assert.True(t, time.Since(startTime) < maxAcceptableTime,
|
|
"Security operations took too long under load")
|
|
}
|
|
```
|
|
|
|
## Testing Infrastructure Setup
|
|
|
|
### Continuous Integration Integration
|
|
```yaml
|
|
# Example CI configuration for new tests
|
|
name: Security Testing
|
|
on: [push, pull_request]
|
|
jobs:
|
|
fuzzing-tests:
|
|
runs-on: ubuntu-latest
|
|
steps:
|
|
- uses: actions/checkout@v3
|
|
- name: Setup Go
|
|
uses: actions/setup-go@v3
|
|
with:
|
|
go-version: '1.21'
|
|
- name: Run Fuzzing Tests
|
|
run: |
|
|
go test -fuzz=. -fuzztime=10s ./pkg/...
|
|
|
|
property-based-tests:
|
|
runs-on: ubuntu-latest
|
|
steps:
|
|
- uses: actions/checkout@v3
|
|
- name: Setup Go
|
|
uses: actions/setup-go@v3
|
|
with:
|
|
go-version: '1.21'
|
|
- name: Run Property Tests
|
|
run: |
|
|
go test -tags=property ./test/property/...
|
|
|
|
performance-tests:
|
|
runs-on: ubuntu-latest
|
|
steps:
|
|
- uses: actions/checkout@v3
|
|
- name: Setup Go
|
|
uses: actions/setup-go@v3
|
|
with:
|
|
go-version: '1.21'
|
|
- name: Run Performance Tests
|
|
run: |
|
|
go test -bench=. -benchmem ./pkg/security/...
|
|
```
|
|
|
|
## Testing Strategy
|
|
- Unit tests for each new testing approach
|
|
- Integration tests to verify testing infrastructure works properly
|
|
- Load testing to ensure tests don't cause system degradation
|
|
- Regular execution of expanded test suite
|
|
|
|
## Code Review Checklist
|
|
- [ ] Fuzzing tests added for critical input parsing functions
|
|
- [ ] Property-based tests implemented for mathematical operations
|
|
- [ ] Integration security test suite created and functional
|
|
- [ ] Performance regression tests established with baselines
|
|
- [ ] Tests are deterministic and reliable
|
|
- [ ] Test coverage metrics improved
|
|
- [ ] Performance impact of tests is acceptable
|
|
|
|
## Rollback Strategy
|
|
If the new testing infrastructure causes issues:
|
|
1. Temporarily disable new test types in CI
|
|
2. Revert to previous test suite while investigating
|
|
3. Address any performance issues with the new tests
|
|
|
|
## Success Metrics
|
|
- Significant increase in test coverage for security-critical code
|
|
- Zero crashes from fuzzing tests in production code paths
|
|
- Property-based tests validate mathematical invariants
|
|
- Security integration tests catch issues before deployment
|
|
- Performance regression tests prevent security feature slowdowns
|
|
- All new tests pass consistently in CI/CD pipeline |