feat: enable attestation queue for validator signing workflow#1204
feat: enable attestation queue for validator signing workflow#1204
Conversation
Adds tn_attestation extension to support data attestation signing. This enables Kuneiform schemas to queue attestation hashes that will be signed by the leader validator. Extension structure: - queue_for_signing() precompile (SYSTEM access only) - Thread-safe attestation queue with deduplication - Leader watch integration (prepared for signing worker) - Comprehensive test coverage resolves: https://github.com/trufnetwork/kwil-db/issues/1620
Time Submission Status
|
WalkthroughAdds a new tn_attestation extension: registers it at startup, provides a precompile Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Caller (Attestation Tx)
participant PC as Precompile: tn_attestation.queue_for_signing
participant P as Proposer Check
participant Q as AttestationQueue
C->>PC: queue_for_signing(attestation_hash)
PC->>PC: validate non-empty
PC->>P: is current node proposer/leader?
alt Node is leader
P-->>PC: yes
PC->>Q: Enqueue(attestation_hash)
Q-->>PC: enqueued / duplicate ignored
else Not leader or unknown proposer
P-->>PC: no/unknown
PC-->>PC: no-op (deterministic)
end
PC-->>C: nil (deterministic)
sequenceDiagram
autonumber
participant S as Startup
participant EXT as tn_attestation.InitializeExtension
participant HK as Engine Ready Hook
participant LW as Leader Watcher
participant Q as AttestationQueue
S->>EXT: call InitializeExtension()
EXT->>HK: hooks.RegisterEngineReadyHook(...)
EXT->>LW: leaderwatch.Register(OnAcquire/OnLose/OnEndBlock)
HK-->>Q: (on ready) log queue size if app present
LW-->>EXT: OnAcquire/OnLose/OnEndBlock callbacks (TODO: start/stop worker, process queue)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧬 Code graph analysis (2)extensions/tn_attestation/tn_attestation.go (3)
extensions/tn_attestation/tn_attestation_test.go (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
extensions/tn_attestation/tn_attestation.go (1)
50-95: Remove commented-out code or document as reference implementation.The leader lifecycle callbacks contain TODO comments with commented-out code (lines 61-62, 76-77, 88-94). Consider either:
- Remove the commented code if the final implementation will differ
- Keep it as reference but add a comment explaining it's a planned implementation
Example:
// onLeaderAcquire is called when the node becomes the leader. -// TODO: Start the signing worker here +// TODO: Start the signing worker here (reference implementation below) func onLeaderAcquire(ctx context.Context, app *common.App, block *common.BlockContext) { if app == nil || app.Service == nil { return } logger := app.Service.Logger.New(ExtensionName) logger.Info("tn_attestation: acquired leadership") - // TODO: Start signing worker + // Reference implementation (to be completed in future PR): // ext := GetExtension() // ext.startSigningWorker() }extensions/tn_attestation/tn_attestation_test.go (6)
184-188: Ensure precompile is registered before use to avoid nil-initializer panicsSubtests rely on "RegistrationSuccess" running first. Running this subtest in isolation can panic. Call a helper to ensure registration.
Apply this diff:
+ ensurePrecompileRegistered(t) // Initialize the precompile initializer := precompiles.RegisteredPrecompiles()[ExtensionName] precompile, err := initializer(context.Background(), app.Service, nil, "", nil) require.NoError(t, err)Add this helper to the file (outside tests):
// test helper to guarantee registration without ordering assumptions func ensurePrecompileRegistered(t *testing.T) { t.Helper() if _, ok := precompiles.RegisteredPrecompiles()[ExtensionName]; !ok { require.NoError(t, registerPrecompile()) } }
232-235: Repeat: ensure precompile registered in each subtestSame robustness concern here.
+ ensurePrecompileRegistered(t) // Initialize the precompile initializer := precompiles.RegisteredPrecompiles()[ExtensionName] precompile, err := initializer(context.Background(), app.Service, nil, "", nil) require.NoError(t, err)
268-271: Repeat: ensure precompile registered in each subtestProtect against isolated subtest runs.
+ ensurePrecompileRegistered(t) // Initialize the precompile initializer := precompiles.RegisteredPrecompiles()[ExtensionName] precompile, err := initializer(context.Background(), app.Service, nil, "", nil) require.NoError(t, err)
329-332: Repeat: ensure precompile registered in each subtestProtect against ordering assumptions.
+ ensurePrecompileRegistered(t) // Initialize the precompile initializer := precompiles.RegisteredPrecompiles()[ExtensionName] precompile, err := initializer(context.Background(), appLeader.Service, nil, "", nil) require.NoError(t, err)
371-374: Repeat: ensure precompile registered in each subtestProtect against ordering assumptions.
+ ensurePrecompileRegistered(t) // Initialize the precompile initializer := precompiles.RegisteredPrecompiles()[ExtensionName] precompile, err := initializer(context.Background(), app.Service, nil, "", nil) require.NoError(t, err)
190-191: Avoid relying onMethods[0]; select method by name "queue_for_signing"Indexing assumes method order and is brittle. Resolve by name to future‑proof tests.
Example pattern:
var handler func(*common.EngineContext, *common.App, []any, func([]any) error) error for _, m := range precompile.Methods { if m.Name == "queue_for_signing" { handler = m.Handler break } } require.NotNil(t, handler)Also applies to: 237-238, 273-274, 333-337, 376-377
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
extensions/register.go(2 hunks)extensions/tn_attestation/constants.go(1 hunks)extensions/tn_attestation/precompile.go(1 hunks)extensions/tn_attestation/queue.go(1 hunks)extensions/tn_attestation/tn_attestation.go(1 hunks)extensions/tn_attestation/tn_attestation_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
extensions/register.go (1)
extensions/tn_attestation/tn_attestation.go (1)
InitializeExtension(16-35)
extensions/tn_attestation/precompile.go (2)
extensions/tn_attestation/constants.go (1)
ExtensionName(3-3)extensions/tn_attestation/queue.go (2)
GetAttestationQueue(86-91)AttestationQueue(7-10)
extensions/tn_attestation/tn_attestation.go (3)
extensions/tn_attestation/constants.go (1)
ExtensionName(3-3)extensions/leaderwatch/leaderwatch.go (2)
Register(149-162)Callbacks(13-17)extensions/tn_attestation/queue.go (1)
GetAttestationQueue(86-91)
extensions/tn_attestation/tn_attestation_test.go (2)
extensions/tn_attestation/queue.go (2)
NewAttestationQueue(13-17)GetAttestationQueue(86-91)extensions/tn_attestation/constants.go (1)
ExtensionName(3-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: acceptance-test
🔇 Additional comments (10)
extensions/register.go (1)
6-6: LGTM! Extension registration follows established pattern.The tn_attestation extension is properly imported and initialized alongside existing extensions.
Also applies to: 17-17
extensions/tn_attestation/constants.go (1)
1-3: LGTM! Clean constant definition.The extension name constant is appropriately defined and will be used consistently throughout the extension.
extensions/tn_attestation/precompile.go (1)
26-65: LGTM! Handler correctly maintains determinism.The handler properly implements the requirements:
- ✅ Validates non-empty attestation hash
- ✅ Defensively checks leadership by comparing proposer identity
- ✅ Only enqueues when node is leader
- ✅ Returns nil for all validators to maintain determinism
- ✅ Includes debug logging when available
The leadership detection logic (lines 38-47) correctly treats missing context as non-leader, ensuring deterministic behavior across all validators.
extensions/tn_attestation/tn_attestation.go (2)
16-35: LGTM! Extension initialization is robust.The initialization sequence properly registers all components with appropriate error handling:
- ✅ Precompile registration
- ✅ Engine ready hook
- ✅ Leader watcher callbacks
The use of
panicfor registration failures is appropriate for initialization-time errors.
38-48: LGTM! Engine ready hook properly logs initialization.The hook safely checks for nil values and logs the queue state, providing useful observability.
extensions/tn_attestation/queue.go (5)
7-17: LGTM! Queue structure and constructor are well-designed.
- ✅ Uses
sync.RWMutexfor efficient read/write concurrency- ✅ Uses
map[string]struct{}for O(1) deduplication (zero memory overhead for values)- ✅ Clean constructor pattern
21-31: LGTM! Enqueue correctly implements deduplication.
- ✅ Proper write locking
- ✅ Returns boolean indicating whether hash was added
- ✅ Efficient O(1) duplicate detection
34-51: LGTM! DequeueAll safely extracts all hashes.
- ✅ Proper write locking
- ✅ Returns
nilfor empty queue (clearer than empty slice)- ✅ Clears queue after dequeuing
54-58: LGTM! Read operations correctly use RLock.Both
Len()andCopy()properly useRLockfor read-only operations, allowing concurrent reads while preventing data races.Also applies to: 68-77
86-91: LGTM! Singleton pattern correctly implemented.
- ✅ Uses
sync.Oncefor thread-safe lazy initialization- ✅ Exported function provides global access for precompile and worker
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
extensions/tn_attestation/tn_attestation.go (1)
39-40: Consider extracting the repeated nil checks.The pattern
if app == nil || app.Service == nil { return ... }appears in four callbacks. While the current code is clear, you could optionally extract this to a helper function to reduce duplication:// validateApp returns false if app or app.Service is nil func validateApp(app *common.App) bool { return app != nil && app.Service != nil }Then use it in the callbacks:
func engineReadyHook(ctx context.Context, app *common.App) error { if !validateApp(app) { return nil } // ... rest of function }However, this is a minor improvement and the current explicit checks are perfectly acceptable.
Also applies to: 53-54, 69-70, 85-86
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
extensions/tn_attestation/tn_attestation.go(1 hunks)extensions/tn_attestation/tn_attestation_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- extensions/tn_attestation/tn_attestation_test.go
🧰 Additional context used
🧬 Code graph analysis (1)
extensions/tn_attestation/tn_attestation.go (3)
extensions/tn_attestation/constants.go (1)
ExtensionName(3-3)extensions/leaderwatch/leaderwatch.go (2)
Register(149-162)Callbacks(13-17)extensions/tn_attestation/queue.go (1)
GetAttestationQueue(86-91)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: acceptance-test
🔇 Additional comments (4)
extensions/tn_attestation/tn_attestation.go (4)
16-35: LGTM! Clean initialization setup.The extension registration is well-structured with clear error messages. Using
panicfor initialization failures is an acceptable pattern in Go, ensuring the node fails fast if critical components cannot be registered.
37-48: LGTM! Well-implemented status hook.The nil checks are appropriate, and logging the queue size provides useful diagnostic information at startup.
50-64: Ensure the TODO is tracked in the project.The placeholder for starting the signing worker is clear, but please verify that this follow-up work is tracked (e.g., in an issue or project board) so the signing workflow can be completed in a future PR.
66-80: Ensure the TODO is tracked in the project.Similar to
onLeaderAcquire, please verify that the signing worker shutdown is tracked for future implementation.
2 participants
Adds tn_attestation extension to support data attestation signing. This enables Kuneiform schemas to queue attestation hashes that will be signed by the leader validator.
Extension structure:
resolves: #1205
Description
Related Problem
Summary by CodeRabbit
New Features
Tests