-
Notifications
You must be signed in to change notification settings - Fork 19
feat: aggsender: send a new certificate as soon latest is settled #1457
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
base: develop
Are you sure you want to change the base?
feat: aggsender: send a new certificate as soon latest is settled #1457
Conversation
8c4b5db to
0680fc0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces a new certificate trigger system for the aggsender that allows configurable certificate generation modes. The changes enable certificates to be generated "as soon as possible" (ASAP) after a successful settlement, rather than only at epoch boundaries.
Changes:
- Introduced a new
TriggerCertModeconfiguration field with support forEpochBased,NewBridge,ASAP, andAutomodes - Moved
EpochNotificationPercentagefrom root config toTriggerEpochBasedsub-config (breaking change) - Refactored trigger implementations into separate files and added a factory pattern for creating trigger instances
Reviewed changes
Copilot reviewed 21 out of 23 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| config/default.go | Updated default configuration to reflect new trigger structure |
| config/config_test.go | Added test for deprecated field EpochNotificationPercentage |
| config/config.go | Added deprecation handling for moved configuration field |
| aggsender/types/interfaces.go | Added trigger mode enum and new OnAggsenderWaitingTrigger method |
| aggsender/trigger/types/mocks/mock_epoch_notifier.go | Fixed import path for epoch notifier types |
| aggsender/trigger/types/epoch_notifier_test.go | Added unit test for EpochStatus.String() |
| aggsender/trigger/types/epoch_notifier.go | Extracted epoch notifier types to separate package |
| aggsender/trigger/trigger_by_epoch_test.go | Moved and updated epoch-based trigger tests |
| aggsender/trigger/trigger_by_epoch.go | Refactored epoch-based trigger to separate file |
| aggsender/trigger/trigger_by_bridge_test.go | Moved and updated bridge trigger tests |
| aggsender/trigger/trigger_by_bridge.go | Extracted bridge trigger to separate file |
| aggsender/trigger/trigger_asap.go | Added new ASAP trigger implementation |
| aggsender/trigger/factory_test.go | Added factory tests for trigger creation |
| aggsender/trigger/factory.go | Added factory for creating trigger instances |
| aggsender/trigger/epoch_notifier_per_block_test.go | Updated package name |
| aggsender/trigger/epoch_notifier_per_block.go | Updated package name and imports |
| aggsender/mocks/mock_certificate_send_trigger.go | Generated mock for new interface method |
| aggsender/config/config_test.go | Updated tests to use new config structure |
| aggsender/config/config.go | Added new trigger configuration fields |
| aggsender/certificate_send_trigger_test.go | Removed (moved to trigger package) |
| aggsender/aggsender_test.go | Updated tests to use new epoch event types |
| aggsender/aggsender.go | Added call to OnAggsenderWaitingTrigger when no pending certificates |
| .mockery.yaml | Added mock generation config for trigger types |
Comments suppressed due to low confidence (2)
aggsender/trigger/trigger_asap.go:70
- The new
asapTriggerimplementation lacks test coverage. Other trigger implementations (epochBasedTriggerandpreconfTrigger) have corresponding test files (trigger_by_epoch_test.goandtrigger_by_bridge_test.go), but there is notrigger_asap_test.gofile. Tests should be added to cover the ASAP trigger's behavior includingTriggerCh,ForceTriggerEvent,OnAggsenderWaitingTrigger, and context cancellation scenarios.
package trigger
import (
"context"
"time"
"github.com/agglayer/aggkit/aggsender/types"
aggkitcommon "github.com/agglayer/aggkit/common"
)
// defaultDelay is the default delay before sending a trigger event
const defaultDelay = 1 * time.Second
type asapTriggerEvent struct{}
func (e *asapTriggerEvent) String() string {
return "ASAP Event"
}
// asapTrigger is a trigger implementation that executes operations as soon as possible.
// It try to generate a new cert after last cert is in a final state (settled or inError)
// you can configure an offset
type asapTrigger struct {
log aggkitcommon.Logger
ch chan types.CertificateTriggerEvent
ctx context.Context
aggsenderBusyState bool
delay time.Duration
}
func newASAPTrigger(log aggkitcommon.Logger) *asapTrigger {
return &asapTrigger{
log: log,
aggsenderBusyState: true,
delay: defaultDelay,
}
}
func (r *asapTrigger) Setup(_ context.Context) {
}
func (r *asapTrigger) Status() string {
return "ASAP Runner: trying to generate certs as soon as possible"
}
func (r *asapTrigger) TriggerCh(ctx context.Context) <-chan types.CertificateTriggerEvent {
ch := make(chan types.CertificateTriggerEvent)
r.ch = ch
r.ctx = ctx
return ch
}
// ForceTriggerEvent forces the preconf trigger to emit a synchronization event.
func (r *asapTrigger) ForceTriggerEvent() {
r.ch <- &asapTriggerEvent{}
}
// OnAggsenderWaitingTrigger Aggsender is waiting for a trigger to generate a new certificate
func (r *asapTrigger) OnAggsenderWaitingTrigger() {
// send a event to channel r.ch after r.delay. Also check if r.ctx is done
r.log.Debugf("ASAP Trigger: sending a trigger in %s", r.delay.String())
go func() {
select {
case <-r.ctx.Done():
return
case <-time.After(r.delay):
r.ch <- &asapTriggerEvent{}
}
}()
}
aggsender/trigger/trigger_by_epoch.go:1
- The comment incorrectly refers to 'preconf trigger' but this is the
epochBasedTriggerimplementation. The comment should be updated to refer to the epoch notifier: 'ForceTriggerEvent forces the epoch notifier to publish an epoch event immediately.' (Note: line 117-119 has the correct comment).
|



🔄 Changes Summary
statuschecker/that now aredebuginstead ofinfoAggSender.EpochNotificationPercentagetoAggSender.TriggerEpochBasedThis field is no longer mandatory and it depens on the trigger mode choosen.
📋 Config Updates
New field
AggSender.TriggerCertModeyou can choose the trigger of new certificate:EpochBased: this is the legacy mode that waits until reach a percentage of a epoch (you can configure here:AggSender.TriggerEpochBased)ASAP: this mode try to generate a new certificate after a sucessful settled certificateNewBridge: Each time that a new bridge is done in L2 it generate a certificate (if it's possible) (experimental)✅ Testing
🐞 Issues
🔗 Related PRs
📝 Notes