v0.7.13 - -t

This commit is contained in:
Your Name
2025-10-13 16:35:26 -04:00
parent e3938a2c85
commit 62e17af311
14 changed files with 1905 additions and 172 deletions

View File

@@ -238,6 +238,162 @@ DEBUG_TRACE("Entering handle_req_message()");
DEBUG_TRACE("Subscription ID validated: %s", sub_id);
DEBUG_TRACE("Exiting handle_req_message()");
```
## Manual Guards for Expensive Operations
### The Problem
Debug macros use **runtime checks**, which means function arguments are always evaluated:
```c
// ❌ BAD: Database query executes even when debug level is 0
DEBUG_LOG("Count: %d", expensive_database_query());
```
The `expensive_database_query()` will **always execute** because function arguments are evaluated before the `if` check inside the macro.
### The Solution: Manual Guards
For expensive operations (database queries, file I/O, complex calculations), use manual guards:
```c
// ✅ GOOD: Query only executes when debugging is enabled
if (g_debug_level >= DEBUG_LEVEL_DEBUG) {
int count = expensive_database_query();
DEBUG_LOG("Count: %d", count);
}
```
### Standardized Comment Format
To make temporary debug guards easy to find and remove, use this standardized format:
```c
// DEBUG_GUARD_START
if (g_debug_level >= DEBUG_LEVEL_DEBUG) {
// Expensive operation here
sqlite3_stmt* stmt;
const char* sql = "SELECT COUNT(*) FROM events";
int count = 0;
if (sqlite3_prepare_v2(g_db, sql, -1, &stmt, NULL) == SQLITE_OK) {
if (sqlite3_step(stmt) == SQLITE_ROW) {
count = sqlite3_column_int(stmt, 0);
}
sqlite3_finalize(stmt);
}
DEBUG_LOG("Event count: %d", count);
}
// DEBUG_GUARD_END
```
### Easy Removal
When you're done debugging, find and remove all temporary guards:
```bash
# Find all debug guards
grep -n "DEBUG_GUARD_START" src/*.c
# Remove guards with sed (between START and END markers)
sed -i '/DEBUG_GUARD_START/,/DEBUG_GUARD_END/d' src/config.c
```
Or use a simple script:
```bash
#!/bin/bash
# remove_debug_guards.sh
for file in src/*.c; do
sed -i '/DEBUG_GUARD_START/,/DEBUG_GUARD_END/d' "$file"
echo "Removed debug guards from $file"
done
```
### When to Use Manual Guards
Use manual guards for:
- ✅ Database queries
- ✅ File I/O operations
- ✅ Network requests
- ✅ Complex calculations
- ✅ Memory allocations for debug data
- ✅ String formatting with multiple operations
Don't need guards for:
- ❌ Simple variable access
- ❌ Basic arithmetic
- ❌ String literals
- ❌ Function calls that are already cheap
### Example: Database Query Guard
```c
// DEBUG_GUARD_START
if (g_debug_level >= DEBUG_LEVEL_DEBUG) {
sqlite3_stmt* count_stmt;
const char* count_sql = "SELECT COUNT(*) FROM config";
int config_count = 0;
if (sqlite3_prepare_v2(g_db, count_sql, -1, &count_stmt, NULL) == SQLITE_OK) {
if (sqlite3_step(count_stmt) == SQLITE_ROW) {
config_count = sqlite3_column_int(count_stmt, 0);
}
sqlite3_finalize(count_stmt);
}
DEBUG_LOG("Config table has %d rows", config_count);
}
// DEBUG_GUARD_END
```
### Example: Complex String Formatting Guard
```c
// DEBUG_GUARD_START
if (g_debug_level >= DEBUG_LEVEL_TRACE) {
char filter_str[1024] = {0};
int offset = 0;
for (int i = 0; i < filter_count && offset < sizeof(filter_str) - 1; i++) {
offset += snprintf(filter_str + offset, sizeof(filter_str) - offset,
"Filter %d: kind=%d, author=%s; ",
i, filters[i].kind, filters[i].author);
}
DEBUG_TRACE("Processing filters: %s", filter_str);
}
// DEBUG_GUARD_END
```
### Alternative: Compile-Time Guards
For permanent debug code that should be completely removed in production builds, use compile-time guards:
```c
#ifdef ENABLE_DEBUG_CODE
// This code is completely removed when ENABLE_DEBUG_CODE is not defined
int count = expensive_database_query();
DEBUG_LOG("Count: %d", count);
#endif
```
Build with debug code:
```bash
make CFLAGS="-DENABLE_DEBUG_CODE"
```
Build without debug code (production):
```bash
make # No debug code compiled in
```
### Best Practices
1. **Always use standardized markers** (`DEBUG_GUARD_START`/`DEBUG_GUARD_END`) for temporary guards
2. **Add a comment** explaining what you're debugging
3. **Remove guards** when debugging is complete
4. **Use compile-time guards** for permanent debug infrastructure
5. **Keep guards simple** - one guard per logical debug operation
## Performance Impact

View File

@@ -0,0 +1,427 @@
# Unified Startup Sequence Design
## Overview
This document describes the new unified startup sequence where all config values are created first, then CLI overrides are applied as a separate atomic operation. This eliminates the current 3-step incremental building process.
## Current Problems
1. **Incremental Config Building**: Config is built in 3 steps:
- Step 1: `populate_default_config_values()` - adds defaults
- Step 2: CLI overrides applied via `update_config_in_table()`
- Step 3: `add_pubkeys_to_config_table()` - adds generated keys
2. **Race Conditions**: Cache can be refreshed between steps, causing incomplete config reads
3. **Complexity**: Multiple code paths for first-time vs restart scenarios
## New Design Principles
1. **Atomic Config Creation**: All config values created in single transaction
2. **Separate Override Phase**: CLI overrides applied after complete config exists
3. **Unified Code Path**: Same logic for first-time and restart scenarios
4. **Cache Safety**: Cache only loaded after config is complete
---
## Scenario 1: First-Time Startup (No Database)
### Sequence
```
1. Key Generation Phase
├─ generate_random_private_key_bytes() → admin_privkey_bytes
├─ nostr_bytes_to_hex() → admin_privkey (hex)
├─ nostr_ec_public_key_from_private_key() → admin_pubkey_bytes
├─ nostr_bytes_to_hex() → admin_pubkey (hex)
├─ generate_random_private_key_bytes() → relay_privkey_bytes
├─ nostr_bytes_to_hex() → relay_privkey (hex)
├─ nostr_ec_public_key_from_private_key() → relay_pubkey_bytes
└─ nostr_bytes_to_hex() → relay_pubkey (hex)
2. Database Creation Phase
├─ create_database_with_relay_pubkey(relay_pubkey)
│ └─ Sets g_database_path = "<relay_pubkey>.db"
└─ init_database(g_database_path)
└─ Creates database with embedded schema (includes config table)
3. Complete Config Population Phase (ATOMIC)
├─ BEGIN TRANSACTION
├─ populate_all_config_values_atomic()
│ ├─ Insert ALL default config values from DEFAULT_CONFIG_VALUES[]
│ ├─ Insert admin_pubkey
│ └─ Insert relay_pubkey
└─ COMMIT TRANSACTION
4. CLI Override Phase (ATOMIC)
├─ BEGIN TRANSACTION
├─ apply_cli_overrides()
│ ├─ IF cli_options.port_override > 0:
│ │ └─ UPDATE config SET value = ? WHERE key = 'relay_port'
│ ├─ IF cli_options.admin_pubkey_override[0]:
│ │ └─ UPDATE config SET value = ? WHERE key = 'admin_pubkey'
│ └─ IF cli_options.relay_privkey_override[0]:
│ └─ UPDATE config SET value = ? WHERE key = 'relay_privkey'
└─ COMMIT TRANSACTION
5. Secure Key Storage Phase
└─ store_relay_private_key(relay_privkey)
└─ INSERT INTO relay_seckey (private_key_hex) VALUES (?)
6. Cache Initialization Phase
└─ refresh_unified_cache_from_table()
└─ Loads complete config into g_unified_cache
```
### Function Call Sequence
```c
// In main.c - first_time_startup branch
if (is_first_time_startup()) {
// 1. Key Generation
first_time_startup_sequence(&cli_options);
// → Generates keys, stores in g_unified_cache
// → Sets g_database_path
// → Does NOT populate config yet
// 2. Database Creation
init_database(g_database_path);
// → Creates database with schema
// 3. Complete Config Population (NEW FUNCTION)
populate_all_config_values_atomic(&cli_options);
// → Inserts ALL defaults + pubkeys in single transaction
// → Does NOT apply CLI overrides yet
// 4. CLI Override Phase (NEW FUNCTION)
apply_cli_overrides_atomic(&cli_options);
// → Updates config table with CLI overrides
// → Separate transaction after complete config exists
// 5. Secure Key Storage
store_relay_private_key(relay_privkey);
// 6. Cache Initialization
refresh_unified_cache_from_table();
}
```
### New Functions Needed
```c
// In config.c
int populate_all_config_values_atomic(const cli_options_t* cli_options) {
// BEGIN TRANSACTION
// Insert ALL defaults from DEFAULT_CONFIG_VALUES[]
// Insert admin_pubkey from g_unified_cache
// Insert relay_pubkey from g_unified_cache
// COMMIT TRANSACTION
return 0;
}
int apply_cli_overrides_atomic(const cli_options_t* cli_options) {
// BEGIN TRANSACTION
// IF port_override: UPDATE config SET value = ? WHERE key = 'relay_port'
// IF admin_pubkey_override: UPDATE config SET value = ? WHERE key = 'admin_pubkey'
// IF relay_privkey_override: UPDATE config SET value = ? WHERE key = 'relay_privkey'
// COMMIT TRANSACTION
// invalidate_config_cache()
return 0;
}
```
---
## Scenario 2: Restart with Existing Database + CLI Options
### Sequence
```
1. Database Discovery Phase
├─ find_existing_db_files() → ["<relay_pubkey>.db"]
├─ extract_pubkey_from_filename() → relay_pubkey
└─ Sets g_database_path = "<relay_pubkey>.db"
2. Database Initialization Phase
└─ init_database(g_database_path)
└─ Opens existing database
3. Config Validation Phase
└─ validate_config_table_completeness()
├─ Check if all required keys exist
└─ IF missing keys: populate_missing_config_values()
4. CLI Override Phase (ATOMIC)
├─ BEGIN TRANSACTION
├─ apply_cli_overrides()
│ └─ UPDATE config SET value = ? WHERE key = ?
└─ COMMIT TRANSACTION
5. Cache Initialization Phase
└─ refresh_unified_cache_from_table()
└─ Loads complete config into g_unified_cache
```
### Function Call Sequence
```c
// In main.c - existing relay branch
else {
// 1. Database Discovery
char** existing_files = find_existing_db_files();
char* relay_pubkey = extract_pubkey_from_filename(existing_files[0]);
startup_existing_relay(relay_pubkey);
// → Sets g_database_path
// 2. Database Initialization
init_database(g_database_path);
// 3. Config Validation (NEW FUNCTION)
validate_config_table_completeness();
// → Checks for missing keys
// → Populates any missing defaults
// 4. CLI Override Phase (REUSE FUNCTION)
if (has_cli_overrides(&cli_options)) {
apply_cli_overrides_atomic(&cli_options);
}
// 5. Cache Initialization
refresh_unified_cache_from_table();
}
```
### New Functions Needed
```c
// In config.c
int validate_config_table_completeness(void) {
// Check if all DEFAULT_CONFIG_VALUES keys exist
// IF missing: populate_missing_config_values()
return 0;
}
int populate_missing_config_values(void) {
// BEGIN TRANSACTION
// For each key in DEFAULT_CONFIG_VALUES:
// IF NOT EXISTS: INSERT INTO config
// COMMIT TRANSACTION
return 0;
}
int has_cli_overrides(const cli_options_t* cli_options) {
return (cli_options->port_override > 0 ||
cli_options->admin_pubkey_override[0] != '\0' ||
cli_options->relay_privkey_override[0] != '\0');
}
```
---
## Scenario 3: Restart with Existing Database + No CLI Options
### Sequence
```
1. Database Discovery Phase
├─ find_existing_db_files() → ["<relay_pubkey>.db"]
├─ extract_pubkey_from_filename() → relay_pubkey
└─ Sets g_database_path = "<relay_pubkey>.db"
2. Database Initialization Phase
└─ init_database(g_database_path)
└─ Opens existing database
3. Config Validation Phase
└─ validate_config_table_completeness()
├─ Check if all required keys exist
└─ IF missing keys: populate_missing_config_values()
4. Cache Initialization Phase (IMMEDIATE)
└─ refresh_unified_cache_from_table()
└─ Loads complete config into g_unified_cache
```
### Function Call Sequence
```c
// In main.c - existing relay branch (no CLI overrides)
else {
// 1. Database Discovery
char** existing_files = find_existing_db_files();
char* relay_pubkey = extract_pubkey_from_filename(existing_files[0]);
startup_existing_relay(relay_pubkey);
// 2. Database Initialization
init_database(g_database_path);
// 3. Config Validation
validate_config_table_completeness();
// 4. Cache Initialization (IMMEDIATE - no overrides to apply)
refresh_unified_cache_from_table();
}
```
---
## Key Improvements
### 1. Atomic Config Creation
**Before:**
```c
populate_default_config_values(); // Step 1
update_config_in_table("relay_port", port_str); // Step 2
add_pubkeys_to_config_table(); // Step 3
```
**After:**
```c
populate_all_config_values_atomic(&cli_options); // Single transaction
apply_cli_overrides_atomic(&cli_options); // Separate transaction
```
### 2. Elimination of Race Conditions
**Before:**
- Cache could refresh between steps 1-3
- Incomplete config could be read
**After:**
- Config created atomically
- Cache only refreshed after complete config exists
### 3. Unified Code Path
**Before:**
- Different logic for first-time vs restart
- `populate_default_config_values()` vs `add_pubkeys_to_config_table()`
**After:**
- Same validation logic for both scenarios
- `validate_config_table_completeness()` handles both cases
### 4. Clear Separation of Concerns
**Before:**
- CLI overrides mixed with default population
- Unclear when overrides are applied
**After:**
- Phase 1: Complete config creation
- Phase 2: CLI overrides (if any)
- Phase 3: Cache initialization
---
## Implementation Changes Required
### 1. New Functions in config.c
```c
// Atomic config population for first-time startup
int populate_all_config_values_atomic(const cli_options_t* cli_options);
// Atomic CLI override application
int apply_cli_overrides_atomic(const cli_options_t* cli_options);
// Config validation for existing databases
int validate_config_table_completeness(void);
int populate_missing_config_values(void);
// Helper function
int has_cli_overrides(const cli_options_t* cli_options);
```
### 2. Modified Functions in config.c
```c
// Simplify to only generate keys and set database path
int first_time_startup_sequence(const cli_options_t* cli_options);
// Remove config population logic
int add_pubkeys_to_config_table(void); // DEPRECATED - logic moved to populate_all_config_values_atomic()
```
### 3. Modified Startup Flow in main.c
```c
// First-time startup
if (is_first_time_startup()) {
first_time_startup_sequence(&cli_options);
init_database(g_database_path);
populate_all_config_values_atomic(&cli_options); // NEW
apply_cli_overrides_atomic(&cli_options); // NEW
store_relay_private_key(relay_privkey);
refresh_unified_cache_from_table();
}
// Existing relay
else {
startup_existing_relay(relay_pubkey);
init_database(g_database_path);
validate_config_table_completeness(); // NEW
if (has_cli_overrides(&cli_options)) {
apply_cli_overrides_atomic(&cli_options); // NEW
}
refresh_unified_cache_from_table();
}
```
---
## Benefits
1. **Atomicity**: Config creation is atomic - no partial states
2. **Simplicity**: Clear phases with single responsibility
3. **Safety**: Cache only loaded after complete config exists
4. **Consistency**: Same validation logic for all scenarios
5. **Maintainability**: Easier to understand and modify
6. **Testability**: Each phase can be tested independently
---
## Migration Path
1. Implement new functions in config.c
2. Update main.c startup flow
3. Test first-time startup scenario
4. Test restart with CLI overrides
5. Test restart without CLI overrides
6. Remove deprecated functions
7. Update documentation
---
## Testing Strategy
### Test Cases
1. **First-time startup with defaults**
- Verify all config values created atomically
- Verify cache loads complete config
2. **First-time startup with port override**
- Verify defaults created first
- Verify port override applied second
- Verify cache reflects override
3. **Restart with complete config**
- Verify no config changes
- Verify cache loads immediately
4. **Restart with missing config keys**
- Verify missing keys populated
- Verify existing keys unchanged
5. **Restart with CLI overrides**
- Verify overrides applied atomically
- Verify cache invalidated and refreshed
### Validation Points
- Config table row count after each phase
- Cache validity state after each phase
- Transaction boundaries (BEGIN/COMMIT)
- Error handling for failed transactions

View File

@@ -0,0 +1,746 @@
# Unified Startup Implementation Plan
## Overview
This document provides a detailed implementation plan for refactoring the startup sequence to use atomic config creation followed by CLI overrides. This plan breaks down the work into discrete, testable steps.
---
## Phase 1: Create New Functions in config.c
### Step 1.1: Implement `populate_all_config_values_atomic()`
**Location**: `src/config.c`
**Purpose**: Create complete config table in single transaction for first-time startup
**Function Signature**:
```c
int populate_all_config_values_atomic(const cli_options_t* cli_options);
```
**Implementation Details**:
```c
int populate_all_config_values_atomic(const cli_options_t* cli_options) {
if (!g_database) {
DEBUG_ERROR("Database not initialized");
return -1;
}
// Begin transaction
char* err_msg = NULL;
int rc = sqlite3_exec(g_database, "BEGIN TRANSACTION;", NULL, NULL, &err_msg);
if (rc != SQLITE_OK) {
DEBUG_ERROR("Failed to begin transaction: %s", err_msg);
sqlite3_free(err_msg);
return -1;
}
// Prepare INSERT statement
sqlite3_stmt* stmt = NULL;
const char* sql = "INSERT INTO config (key, value) VALUES (?, ?)";
rc = sqlite3_prepare_v2(g_database, sql, -1, &stmt, NULL);
if (rc != SQLITE_OK) {
DEBUG_ERROR("Failed to prepare statement: %s", sqlite3_errmsg(g_database));
sqlite3_exec(g_database, "ROLLBACK;", NULL, NULL, NULL);
return -1;
}
// Insert all default config values
for (size_t i = 0; i < sizeof(DEFAULT_CONFIG_VALUES) / sizeof(DEFAULT_CONFIG_VALUES[0]); i++) {
sqlite3_reset(stmt);
sqlite3_bind_text(stmt, 1, DEFAULT_CONFIG_VALUES[i].key, -1, SQLITE_STATIC);
sqlite3_bind_text(stmt, 2, DEFAULT_CONFIG_VALUES[i].value, -1, SQLITE_STATIC);
rc = sqlite3_step(stmt);
if (rc != SQLITE_DONE) {
DEBUG_ERROR("Failed to insert config key '%s': %s",
DEFAULT_CONFIG_VALUES[i].key, sqlite3_errmsg(g_database));
sqlite3_finalize(stmt);
sqlite3_exec(g_database, "ROLLBACK;", NULL, NULL, NULL);
return -1;
}
}
// Insert admin_pubkey from cache
sqlite3_reset(stmt);
sqlite3_bind_text(stmt, 1, "admin_pubkey", -1, SQLITE_STATIC);
sqlite3_bind_text(stmt, 2, g_unified_cache.admin_pubkey, -1, SQLITE_STATIC);
rc = sqlite3_step(stmt);
if (rc != SQLITE_DONE) {
DEBUG_ERROR("Failed to insert admin_pubkey: %s", sqlite3_errmsg(g_database));
sqlite3_finalize(stmt);
sqlite3_exec(g_database, "ROLLBACK;", NULL, NULL, NULL);
return -1;
}
// Insert relay_pubkey from cache
sqlite3_reset(stmt);
sqlite3_bind_text(stmt, 1, "relay_pubkey", -1, SQLITE_STATIC);
sqlite3_bind_text(stmt, 2, g_unified_cache.relay_pubkey, -1, SQLITE_STATIC);
rc = sqlite3_step(stmt);
if (rc != SQLITE_DONE) {
DEBUG_ERROR("Failed to insert relay_pubkey: %s", sqlite3_errmsg(g_database));
sqlite3_finalize(stmt);
sqlite3_exec(g_database, "ROLLBACK;", NULL, NULL, NULL);
return -1;
}
sqlite3_finalize(stmt);
// Commit transaction
rc = sqlite3_exec(g_database, "COMMIT;", NULL, NULL, &err_msg);
if (rc != SQLITE_OK) {
DEBUG_ERROR("Failed to commit transaction: %s", err_msg);
sqlite3_free(err_msg);
sqlite3_exec(g_database, "ROLLBACK;", NULL, NULL, NULL);
return -1;
}
DEBUG_INFO("Successfully populated all config values atomically");
return 0;
}
```
**Testing**:
- Verify transaction atomicity (all or nothing)
- Verify all DEFAULT_CONFIG_VALUES inserted
- Verify admin_pubkey and relay_pubkey inserted
- Verify error handling on failure
---
### Step 1.2: Implement `apply_cli_overrides_atomic()`
**Location**: `src/config.c`
**Purpose**: Apply CLI overrides to existing config table in single transaction
**Function Signature**:
```c
int apply_cli_overrides_atomic(const cli_options_t* cli_options);
```
**Implementation Details**:
```c
int apply_cli_overrides_atomic(const cli_options_t* cli_options) {
if (!g_database) {
DEBUG_ERROR("Database not initialized");
return -1;
}
if (!cli_options) {
DEBUG_ERROR("CLI options is NULL");
return -1;
}
// Check if any overrides exist
bool has_overrides = false;
if (cli_options->port_override > 0) has_overrides = true;
if (cli_options->admin_pubkey_override[0] != '\0') has_overrides = true;
if (cli_options->relay_privkey_override[0] != '\0') has_overrides = true;
if (!has_overrides) {
DEBUG_INFO("No CLI overrides to apply");
return 0;
}
// Begin transaction
char* err_msg = NULL;
int rc = sqlite3_exec(g_database, "BEGIN TRANSACTION;", NULL, NULL, &err_msg);
if (rc != SQLITE_OK) {
DEBUG_ERROR("Failed to begin transaction: %s", err_msg);
sqlite3_free(err_msg);
return -1;
}
// Prepare UPDATE statement
sqlite3_stmt* stmt = NULL;
const char* sql = "UPDATE config SET value = ? WHERE key = ?";
rc = sqlite3_prepare_v2(g_database, sql, -1, &stmt, NULL);
if (rc != SQLITE_OK) {
DEBUG_ERROR("Failed to prepare statement: %s", sqlite3_errmsg(g_database));
sqlite3_exec(g_database, "ROLLBACK;", NULL, NULL, NULL);
return -1;
}
// Apply port override
if (cli_options->port_override > 0) {
char port_str[16];
snprintf(port_str, sizeof(port_str), "%d", cli_options->port_override);
sqlite3_reset(stmt);
sqlite3_bind_text(stmt, 1, port_str, -1, SQLITE_TRANSIENT);
sqlite3_bind_text(stmt, 2, "relay_port", -1, SQLITE_STATIC);
rc = sqlite3_step(stmt);
if (rc != SQLITE_DONE) {
DEBUG_ERROR("Failed to update relay_port: %s", sqlite3_errmsg(g_database));
sqlite3_finalize(stmt);
sqlite3_exec(g_database, "ROLLBACK;", NULL, NULL, NULL);
return -1;
}
DEBUG_INFO("Applied CLI override: relay_port = %s", port_str);
}
// Apply admin_pubkey override
if (cli_options->admin_pubkey_override[0] != '\0') {
sqlite3_reset(stmt);
sqlite3_bind_text(stmt, 1, cli_options->admin_pubkey_override, -1, SQLITE_STATIC);
sqlite3_bind_text(stmt, 2, "admin_pubkey", -1, SQLITE_STATIC);
rc = sqlite3_step(stmt);
if (rc != SQLITE_DONE) {
DEBUG_ERROR("Failed to update admin_pubkey: %s", sqlite3_errmsg(g_database));
sqlite3_finalize(stmt);
sqlite3_exec(g_database, "ROLLBACK;", NULL, NULL, NULL);
return -1;
}
DEBUG_INFO("Applied CLI override: admin_pubkey");
}
// Apply relay_privkey override
if (cli_options->relay_privkey_override[0] != '\0') {
sqlite3_reset(stmt);
sqlite3_bind_text(stmt, 1, cli_options->relay_privkey_override, -1, SQLITE_STATIC);
sqlite3_bind_text(stmt, 2, "relay_privkey", -1, SQLITE_STATIC);
rc = sqlite3_step(stmt);
if (rc != SQLITE_DONE) {
DEBUG_ERROR("Failed to update relay_privkey: %s", sqlite3_errmsg(g_database));
sqlite3_finalize(stmt);
sqlite3_exec(g_database, "ROLLBACK;", NULL, NULL, NULL);
return -1;
}
DEBUG_INFO("Applied CLI override: relay_privkey");
}
sqlite3_finalize(stmt);
// Commit transaction
rc = sqlite3_exec(g_database, "COMMIT;", NULL, NULL, &err_msg);
if (rc != SQLITE_OK) {
DEBUG_ERROR("Failed to commit transaction: %s", err_msg);
sqlite3_free(err_msg);
sqlite3_exec(g_database, "ROLLBACK;", NULL, NULL, NULL);
return -1;
}
// Invalidate cache to force refresh
invalidate_config_cache();
DEBUG_INFO("Successfully applied CLI overrides atomically");
return 0;
}
```
**Testing**:
- Verify transaction atomicity
- Verify each override type (port, admin_pubkey, relay_privkey)
- Verify cache invalidation after overrides
- Verify no-op when no overrides present
---
### Step 1.3: Implement `validate_config_table_completeness()`
**Location**: `src/config.c`
**Purpose**: Validate config table has all required keys, populate missing ones
**Function Signature**:
```c
int validate_config_table_completeness(void);
```
**Implementation Details**:
```c
int validate_config_table_completeness(void) {
if (!g_database) {
DEBUG_ERROR("Database not initialized");
return -1;
}
DEBUG_INFO("Validating config table completeness");
// Check each default config key
for (size_t i = 0; i < sizeof(DEFAULT_CONFIG_VALUES) / sizeof(DEFAULT_CONFIG_VALUES[0]); i++) {
const char* key = DEFAULT_CONFIG_VALUES[i].key;
// Check if key exists
sqlite3_stmt* stmt = NULL;
const char* sql = "SELECT COUNT(*) FROM config WHERE key = ?";
int rc = sqlite3_prepare_v2(g_database, sql, -1, &stmt, NULL);
if (rc != SQLITE_OK) {
DEBUG_ERROR("Failed to prepare statement: %s", sqlite3_errmsg(g_database));
return -1;
}
sqlite3_bind_text(stmt, 1, key, -1, SQLITE_STATIC);
rc = sqlite3_step(stmt);
int count = 0;
if (rc == SQLITE_ROW) {
count = sqlite3_column_int(stmt, 0);
}
sqlite3_finalize(stmt);
// If key missing, populate it
if (count == 0) {
DEBUG_WARN("Config key '%s' missing, populating with default", key);
rc = populate_missing_config_key(key, DEFAULT_CONFIG_VALUES[i].value);
if (rc != 0) {
DEBUG_ERROR("Failed to populate missing key '%s'", key);
return -1;
}
}
}
DEBUG_INFO("Config table validation complete");
return 0;
}
```
**Helper Function**:
```c
static int populate_missing_config_key(const char* key, const char* value) {
sqlite3_stmt* stmt = NULL;
const char* sql = "INSERT INTO config (key, value) VALUES (?, ?)";
int rc = sqlite3_prepare_v2(g_database, sql, -1, &stmt, NULL);
if (rc != SQLITE_OK) {
DEBUG_ERROR("Failed to prepare statement: %s", sqlite3_errmsg(g_database));
return -1;
}
sqlite3_bind_text(stmt, 1, key, -1, SQLITE_STATIC);
sqlite3_bind_text(stmt, 2, value, -1, SQLITE_STATIC);
rc = sqlite3_step(stmt);
sqlite3_finalize(stmt);
if (rc != SQLITE_DONE) {
DEBUG_ERROR("Failed to insert config key '%s': %s", key, sqlite3_errmsg(g_database));
return -1;
}
return 0;
}
```
**Testing**:
- Verify detection of missing keys
- Verify population of missing keys with defaults
- Verify no changes when all keys present
- Verify error handling
---
### Step 1.4: Implement `has_cli_overrides()`
**Location**: `src/config.c`
**Purpose**: Check if any CLI overrides are present
**Function Signature**:
```c
bool has_cli_overrides(const cli_options_t* cli_options);
```
**Implementation Details**:
```c
bool has_cli_overrides(const cli_options_t* cli_options) {
if (!cli_options) {
return false;
}
return (cli_options->port_override > 0 ||
cli_options->admin_pubkey_override[0] != '\0' ||
cli_options->relay_privkey_override[0] != '\0');
}
```
**Testing**:
- Verify returns true when any override present
- Verify returns false when no overrides
- Verify NULL safety
---
## Phase 2: Update Function Declarations in config.h
### Step 2.1: Add New Function Declarations
**Location**: `src/config.h`
**Changes**:
```c
// Add after existing function declarations
// Atomic config population for first-time startup
int populate_all_config_values_atomic(const cli_options_t* cli_options);
// Atomic CLI override application
int apply_cli_overrides_atomic(const cli_options_t* cli_options);
// Config validation for existing databases
int validate_config_table_completeness(void);
// Helper function to check for CLI overrides
bool has_cli_overrides(const cli_options_t* cli_options);
```
---
## Phase 3: Refactor Startup Flow in main.c
### Step 3.1: Update First-Time Startup Branch
**Location**: `src/main.c` (around lines 1624-1740)
**Current Code**:
```c
if (is_first_time_startup()) {
first_time_startup_sequence(&cli_options);
init_database(g_database_path);
// Current incremental approach
populate_default_config_values();
if (cli_options.port_override > 0) {
char port_str[16];
snprintf(port_str, sizeof(port_str), "%d", cli_options.port_override);
update_config_in_table("relay_port", port_str);
}
add_pubkeys_to_config_table();
store_relay_private_key(relay_privkey);
refresh_unified_cache_from_table();
}
```
**New Code**:
```c
if (is_first_time_startup()) {
// 1. Generate keys and set database path
first_time_startup_sequence(&cli_options);
// 2. Create database with schema
init_database(g_database_path);
// 3. Populate ALL config values atomically (defaults + pubkeys)
if (populate_all_config_values_atomic(&cli_options) != 0) {
DEBUG_ERROR("Failed to populate config values");
return EXIT_FAILURE;
}
// 4. Apply CLI overrides atomically (separate transaction)
if (apply_cli_overrides_atomic(&cli_options) != 0) {
DEBUG_ERROR("Failed to apply CLI overrides");
return EXIT_FAILURE;
}
// 5. Store relay private key securely
store_relay_private_key(relay_privkey);
// 6. Load complete config into cache
refresh_unified_cache_from_table();
}
```
**Testing**:
- Verify first-time startup creates complete config
- Verify CLI overrides applied correctly
- Verify cache loads complete config
- Verify error handling at each step
---
### Step 3.2: Update Existing Relay Startup Branch
**Location**: `src/main.c` (around lines 1741-1928)
**Current Code**:
```c
else {
char** existing_files = find_existing_db_files();
char* relay_pubkey = extract_pubkey_from_filename(existing_files[0]);
startup_existing_relay(relay_pubkey);
init_database(g_database_path);
// Current approach - unclear when overrides applied
populate_default_config_values();
if (cli_options.port_override > 0) {
// ... override logic ...
}
refresh_unified_cache_from_table();
}
```
**New Code**:
```c
else {
// 1. Discover existing database
char** existing_files = find_existing_db_files();
if (!existing_files || !existing_files[0]) {
DEBUG_ERROR("No existing database files found");
return EXIT_FAILURE;
}
char* relay_pubkey = extract_pubkey_from_filename(existing_files[0]);
startup_existing_relay(relay_pubkey);
// 2. Open existing database
init_database(g_database_path);
// 3. Validate config table completeness (populate missing keys)
if (validate_config_table_completeness() != 0) {
DEBUG_ERROR("Failed to validate config table");
return EXIT_FAILURE;
}
// 4. Apply CLI overrides if present (separate transaction)
if (has_cli_overrides(&cli_options)) {
if (apply_cli_overrides_atomic(&cli_options) != 0) {
DEBUG_ERROR("Failed to apply CLI overrides");
return EXIT_FAILURE;
}
}
// 5. Load complete config into cache
refresh_unified_cache_from_table();
}
```
**Testing**:
- Verify existing relay startup with complete config
- Verify missing keys populated
- Verify CLI overrides applied when present
- Verify no changes when no overrides
- Verify cache loads correctly
---
## Phase 4: Deprecate Old Functions
### Step 4.1: Mark Functions as Deprecated
**Location**: `src/config.c`
**Functions to Deprecate**:
1. `populate_default_config_values()` - replaced by `populate_all_config_values_atomic()`
2. `add_pubkeys_to_config_table()` - logic moved to `populate_all_config_values_atomic()`
**Changes**:
```c
// Mark as deprecated in comments
// DEPRECATED: Use populate_all_config_values_atomic() instead
// This function will be removed in a future version
int populate_default_config_values(void) {
// ... existing implementation ...
}
// DEPRECATED: Use populate_all_config_values_atomic() instead
// This function will be removed in a future version
int add_pubkeys_to_config_table(void) {
// ... existing implementation ...
}
```
---
## Phase 5: Testing Strategy
### Unit Tests
1. **Test `populate_all_config_values_atomic()`**
- Test with valid cli_options
- Test transaction rollback on error
- Test all config keys inserted
- Test pubkeys inserted correctly
2. **Test `apply_cli_overrides_atomic()`**
- Test port override
- Test admin_pubkey override
- Test relay_privkey override
- Test multiple overrides
- Test no overrides
- Test transaction rollback on error
3. **Test `validate_config_table_completeness()`**
- Test with complete config
- Test with missing keys
- Test population of missing keys
4. **Test `has_cli_overrides()`**
- Test with each override type
- Test with no overrides
- Test with NULL cli_options
### Integration Tests
1. **First-Time Startup**
```bash
# Clean environment
rm -f *.db
# Start relay with defaults
./build/c_relay_x86
# Verify config table complete
sqlite3 <relay_pubkey>.db "SELECT COUNT(*) FROM config;"
# Expected: 20+ rows (all defaults + pubkeys)
# Verify cache loaded
# Check relay.log for cache refresh message
```
2. **First-Time Startup with CLI Overrides**
```bash
# Clean environment
rm -f *.db
# Start relay with port override
./build/c_relay_x86 --port 9999
# Verify port override applied
sqlite3 <relay_pubkey>.db "SELECT value FROM config WHERE key='relay_port';"
# Expected: 9999
```
3. **Restart with Existing Database**
```bash
# Start relay (creates database)
./build/c_relay_x86
# Stop relay
pkill -f c_relay_
# Restart relay
./build/c_relay_x86
# Verify config unchanged
# Check relay.log for validation message
```
4. **Restart with CLI Overrides**
```bash
# Start relay (creates database)
./build/c_relay_x86
# Stop relay
pkill -f c_relay_
# Restart with port override
./build/c_relay_x86 --port 9999
# Verify port override applied
sqlite3 <relay_pubkey>.db "SELECT value FROM config WHERE key='relay_port';"
# Expected: 9999
```
### Regression Tests
Run existing test suite to ensure no breakage:
```bash
./tests/run_all_tests.sh
```
---
## Phase 6: Documentation Updates
### Files to Update
1. **docs/configuration_guide.md**
- Update startup sequence description
- Document new atomic config creation
- Document CLI override behavior
2. **docs/startup_flows_complete.md**
- Update with new flow diagrams
- Document new function calls
3. **README.md**
- Update CLI options documentation
- Document override behavior
---
## Implementation Timeline
### Week 1: Core Functions
- Day 1-2: Implement `populate_all_config_values_atomic()`
- Day 3-4: Implement `apply_cli_overrides_atomic()`
- Day 5: Implement `validate_config_table_completeness()` and `has_cli_overrides()`
### Week 2: Integration
- Day 1-2: Update main.c startup flow
- Day 3-4: Testing and bug fixes
- Day 5: Documentation updates
### Week 3: Cleanup
- Day 1-2: Deprecate old functions
- Day 3-4: Final testing and validation
- Day 5: Code review and merge
---
## Risk Mitigation
### Potential Issues
1. **Database Lock Contention**
- Risk: Multiple transactions could cause locks
- Mitigation: Use BEGIN IMMEDIATE for write transactions
2. **Cache Invalidation Timing**
- Risk: Cache could be read before overrides applied
- Mitigation: Invalidate cache immediately after overrides
3. **Backward Compatibility**
- Risk: Existing databases might have incomplete config
- Mitigation: `validate_config_table_completeness()` handles this
4. **Transaction Rollback**
- Risk: Partial config on error
- Mitigation: All operations in transactions with proper rollback
---
## Success Criteria
1. ✅ All config values created atomically in first-time startup
2. ✅ CLI overrides applied in separate atomic transaction
3. ✅ Existing databases validated and missing keys populated
4. ✅ Cache only loaded after complete config exists
5. ✅ All existing tests pass
6. ✅ No race conditions in config creation
7. ✅ Clear separation between config creation and override phases
---
## Rollback Plan
If issues arise during implementation:
1. **Revert main.c changes** - restore original startup flow
2. **Keep new functions** - they can coexist with old code
3. **Add feature flag** - allow toggling between old and new behavior
4. **Gradual migration** - enable new behavior per scenario
```c
// Feature flag approach
#define USE_ATOMIC_CONFIG_CREATION 1
#if USE_ATOMIC_CONFIG_CREATION
// New atomic approach
populate_all_config_values_atomic(&cli_options);
apply_cli_overrides_atomic(&cli_options);
#else
// Old incremental approach
populate_default_config_values();
// ... existing code ...
#endif
```