Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: comments
Browse files Browse the repository at this point in the history
@goran-ethernal
goran-ethernal committed Jan 31, 2025
1 parent ebe94cd commit 91a3f7e
Showing 11 changed files with 233 additions and 221 deletions.
4 changes: 0 additions & 4 deletions .mockery.yaml
View file
Original file line number Diff line number Diff line change
@@ -43,10 +43,6 @@ packages:
github.com/agglayer/aggkit/aggsender/types:
config:
all: true
github.com/agglayer/aggkit/aggsender:
config:
dir: "{{ .InterfaceDir }}/mocks"
all: true
github.com/agglayer/aggkit/bridgesync:
config:
dir: "{{ .InterfaceDir }}/mocks"
16 changes: 10 additions & 6 deletions aggsender/aggsender.go
View file
Original file line number Diff line number Diff line change
@@ -49,7 +49,7 @@ type AggSender struct {

status types.AggsenderStatus

flowManager FlowManager
flow types.AggsenderFlow
}

// New returns a new AggSender
@@ -77,10 +77,14 @@ func New(

var (
aggchainProofClient grpc.AggchainProofClientInterface
flowManager FlowManager
flowManager types.AggsenderFlow
)

if cfg.AggchainProofClientURL != "" {
if types.AggsenderMode(cfg.Mode) == types.AggchainProverMode {
if cfg.AggchainProofClientURL == "" {
return nil, fmt.Errorf("aggchain prover mode requires AggchainProofClientURL")
}

aggchainProofClient, err = grpc.NewAggchainProofClient(cfg.AggchainProofClientURL)
if err != nil {
return nil, fmt.Errorf("error creating aggkit prover client: %w", err)
@@ -103,7 +107,7 @@ func New(
aggsenderKey: sequencerPrivateKey,
epochNotifier: epochNotifier,
status: types.AggsenderStatus{Status: types.StatusNone},
flowManager: flowManager,
flow: flowManager,
}, nil
}

@@ -203,7 +207,7 @@ func (a *AggSender) sendCertificate(ctx context.Context) (*agglayer.SignedCertif
return nil, nil
}

certificateParams, err := a.flowManager.GetCertificateBuildParams(ctx)
certificateParams, err := a.flow.GetCertificateBuildParams(ctx)
if err != nil {
return nil, fmt.Errorf("error getting certificate build params: %w", err)
}
@@ -212,7 +216,7 @@ func (a *AggSender) sendCertificate(ctx context.Context) (*agglayer.SignedCertif
return nil, nil
}

certificate, err := a.flowManager.BuildCertificate(ctx, certificateParams)
certificate, err := a.flow.BuildCertificate(ctx, certificateParams)
if err != nil {
return nil, fmt.Errorf("error building certificate: %w", err)
}
32 changes: 17 additions & 15 deletions aggsender/aggsender_test.go
View file
Original file line number Diff line number Diff line change
@@ -43,6 +43,7 @@ func TestConfigString(t *testing.T) {
BlockFinality: "latestBlock",
EpochNotificationPercentage: 50,
SaveCertificatesToFilesPath: "/path/to/certificates",
Mode: "PP",
}

expected := "StoragePath: /path/to/storage\n" +
@@ -59,7 +60,8 @@ func TestConfigString(t *testing.T) {
"BridgeMetadataAsHash: false\n" +
"DryRun: false\n" +
"EnableRPC: false\n" +
"AggchainProofClientURL: \n"
"AggchainProofClientURL: \n" +
"Mode: PP\n"

require.Equal(t, expected, config.String())
}
@@ -414,7 +416,7 @@ func TestSendCertificate_NoClaims(t *testing.T) {
l1infoTreeSyncer: mockL1InfoTreeSyncer,
aggsenderKey: privateKey,
cfg: Config{},
flowManager: newPPFlow(logger, Config{}, mockStorage, nil, mockL2Syncer),
flow: newPPFlow(logger, Config{}, mockStorage, nil, mockL2Syncer),
}

mockStorage.On("GetCertificatesByStatus", agglayer.NonSettledStatuses).Return([]*aggsendertypes.CertificateInfo{}, nil).Once()
@@ -696,13 +698,13 @@ func TestSendCertificate(t *testing.T) {

testCases := []struct {
name string
mockFn func(*mocks.AggSenderStorage, *mocks.FlowManager, *mocks.L1InfoTreeSyncer, *agglayer.AgglayerClientMock)
mockFn func(*mocks.AggSenderStorage, *mocks.AggsenderFlow, *mocks.L1InfoTreeSyncer, *agglayer.AgglayerClientMock)
expectedError string
}{
{
name: "error getting pending certificates",
mockFn: func(mockStorage *mocks.AggSenderStorage,
mockFlow *mocks.FlowManager,
mockFlow *mocks.AggsenderFlow,
mockL1InfoTreeSyncer *mocks.L1InfoTreeSyncer,
mockAgglayerClient *agglayer.AgglayerClientMock) {
mockStorage.On("GetCertificatesByStatus", agglayer.NonSettledStatuses).Return(nil, errors.New("some error")).Once()
@@ -712,7 +714,7 @@ func TestSendCertificate(t *testing.T) {
{
name: "has pending certificates",
mockFn: func(mockStorage *mocks.AggSenderStorage,
mockFlow *mocks.FlowManager,
mockFlow *mocks.AggsenderFlow,
mockL1InfoTreeSyncer *mocks.L1InfoTreeSyncer,
mockAgglayerClient *agglayer.AgglayerClientMock) {
mockStorage.On("GetCertificatesByStatus", agglayer.NonSettledStatuses).Return(
@@ -727,7 +729,7 @@ func TestSendCertificate(t *testing.T) {
{
name: "error getting certificate build params",
mockFn: func(mockStorage *mocks.AggSenderStorage,
mockFlow *mocks.FlowManager,
mockFlow *mocks.AggsenderFlow,
mockL1InfoTreeSyncer *mocks.L1InfoTreeSyncer,
mockAgglayerClient *agglayer.AgglayerClientMock) {
mockStorage.On("GetCertificatesByStatus", agglayer.NonSettledStatuses).Return([]*aggsendertypes.CertificateInfo{}, nil).Once()
@@ -738,7 +740,7 @@ func TestSendCertificate(t *testing.T) {
{
name: "no consumed bridges",
mockFn: func(mockStorage *mocks.AggSenderStorage,
mockFlow *mocks.FlowManager,
mockFlow *mocks.AggsenderFlow,
mockL1InfoTreeSyncer *mocks.L1InfoTreeSyncer,
mockAgglayerClient *agglayer.AgglayerClientMock) {
mockStorage.On("GetCertificatesByStatus", agglayer.NonSettledStatuses).Return([]*aggsendertypes.CertificateInfo{}, nil).Once()
@@ -750,7 +752,7 @@ func TestSendCertificate(t *testing.T) {
{
name: "error building certificate",
mockFn: func(mockStorage *mocks.AggSenderStorage,
mockFlow *mocks.FlowManager,
mockFlow *mocks.AggsenderFlow,
mockL1InfoTreeSyncer *mocks.L1InfoTreeSyncer,
mockAgglayerClient *agglayer.AgglayerClientMock) {
mockStorage.On("GetCertificatesByStatus", agglayer.NonSettledStatuses).Return([]*aggsendertypes.CertificateInfo{}, nil).Once()
@@ -764,7 +766,7 @@ func TestSendCertificate(t *testing.T) {
{
name: "error sending certificate",
mockFn: func(mockStorage *mocks.AggSenderStorage,
mockFlow *mocks.FlowManager,
mockFlow *mocks.AggsenderFlow,
mockL1InfoTreeSyncer *mocks.L1InfoTreeSyncer,
mockAgglayerClient *agglayer.AgglayerClientMock) {
mockStorage.On("GetCertificatesByStatus", agglayer.NonSettledStatuses).Return([]*aggsendertypes.CertificateInfo{}, nil).Once()
@@ -784,7 +786,7 @@ func TestSendCertificate(t *testing.T) {
{
name: "error saving certificate to storage",
mockFn: func(mockStorage *mocks.AggSenderStorage,
mockFlow *mocks.FlowManager,
mockFlow *mocks.AggsenderFlow,
mockL1InfoTreeSyncer *mocks.L1InfoTreeSyncer,
mockAgglayerClient *agglayer.AgglayerClientMock) {
mockStorage.On("GetCertificatesByStatus", agglayer.NonSettledStatuses).Return([]*aggsendertypes.CertificateInfo{}, nil).Once()
@@ -805,7 +807,7 @@ func TestSendCertificate(t *testing.T) {
{
name: "successful sending and saving of a certificate",
mockFn: func(mockStorage *mocks.AggSenderStorage,
mockFlow *mocks.FlowManager,
mockFlow *mocks.AggsenderFlow,
mockL1InfoTreeSyncer *mocks.L1InfoTreeSyncer,
mockAgglayerClient *agglayer.AgglayerClientMock) {
mockStorage.On("GetCertificatesByStatus", agglayer.NonSettledStatuses).Return([]*aggsendertypes.CertificateInfo{}, nil).Once()
@@ -831,16 +833,16 @@ func TestSendCertificate(t *testing.T) {
t.Parallel()

mockStorage := mocks.NewAggSenderStorage(t)
mockFlowManager := mocks.NewFlowManager(t)
mockAggsenderFlow := mocks.NewAggsenderFlow(t)
mockL1InfoTreeSyncer := mocks.NewL1InfoTreeSyncer(t)
mockAgglayerClient := agglayer.NewAgglayerClientMock(t)
tt.mockFn(mockStorage, mockFlowManager, mockL1InfoTreeSyncer, mockAgglayerClient)
tt.mockFn(mockStorage, mockAggsenderFlow, mockL1InfoTreeSyncer, mockAgglayerClient)

aggsender := &AggSender{
log: log.WithFields("aggsender-test", "sendCertificate"),
aggsenderKey: privateKey,
storage: mockStorage,
flowManager: mockFlowManager,
flow: mockAggsenderFlow,
aggLayerClient: mockAgglayerClient,
l1infoTreeSyncer: mockL1InfoTreeSyncer,
cfg: Config{
@@ -857,7 +859,7 @@ func TestSendCertificate(t *testing.T) {
}

mockStorage.AssertExpectations(t)
mockFlowManager.AssertExpectations(t)
mockAggsenderFlow.AssertExpectations(t)
mockL1InfoTreeSyncer.AssertExpectations(t)
})
}
6 changes: 4 additions & 2 deletions aggsender/config.go
View file
Original file line number Diff line number Diff line change
@@ -25,7 +25,6 @@ type Config struct {
EpochNotificationPercentage uint `mapstructure:"EpochNotificationPercentage"`
// SaveCertificatesToFilesPath if != "" tells the AggSender to save the certificates to a file in this path
SaveCertificatesToFilesPath string `mapstructure:"SaveCertificatesToFilesPath"`

// MaxRetriesStoreCertificate is the maximum number of retries to store a certificate
// 0 is infinite
MaxRetriesStoreCertificate int `mapstructure:"MaxRetriesStoreCertificate"`
@@ -46,6 +45,8 @@ type Config struct {
EnableRPC bool `mapstructure:"EnableRPC"`
// AggchainProofClientURL is the URL of the AggchainProofClient
AggchainProofClientURL string `mapstructure:"AggchainProofClientURL"`
// Mode is the mode of the AggSender (regular pessimistic proof mode or the aggchain prover mode)
Mode string `jsonschema:"enum=PP, enum=AggchainProver" mapstructure:"Mode"` //nolint:lll

Check failure on line 49 in aggsender/config.go

GitHub Actions / lint 

directive `//nolint:lll` is unused for linter "lll" (nolintlint)
}

// String returns a string representation of the Config
@@ -64,5 +65,6 @@ func (c Config) String() string {
"BridgeMetadataAsHash: " + fmt.Sprintf("%t", c.BridgeMetadataAsHash) + "\n" +
"DryRun: " + fmt.Sprintf("%t", c.DryRun) + "\n" +
"EnableRPC: " + fmt.Sprintf("%t", c.EnableRPC) + "\n" +
"AggchainProofClientURL: " + c.AggchainProofClientURL + "\n"
"AggchainProofClientURL: " + c.AggchainProofClientURL + "\n" +
"Mode: " + c.Mode + "\n"
}
18 changes: 9 additions & 9 deletions aggsender/flow_aggchain_prover.go
View file
Original file line number Diff line number Diff line change
@@ -12,7 +12,7 @@ import (

// aggchainProverFlow is a struct that holds the logic for the AggchainProver prover type flow
type aggchainProverFlow struct {
*flowManager
*baseFlow

aggchainProofClient grpc.AggchainProofClientInterface
}
@@ -26,7 +26,7 @@ func newAggchainProverFlow(log types.Logger,
l2Syncer types.L2BridgeSyncer) *aggchainProverFlow {
return &aggchainProverFlow{
aggchainProofClient: aggkitProverClient,
flowManager: &flowManager{
baseFlow: &baseFlow{
log: log,
cfg: cfg,
l2Syncer: l2Syncer,
@@ -43,34 +43,34 @@ func newAggchainProverFlow(log types.Logger,
func (a *aggchainProverFlow) GetCertificateBuildParams(ctx context.Context) (*types.CertificateBuildParams, error) {
lastSentCertificateInfo, err := a.storage.GetLastSentCertificate()
if err != nil {
return nil, err
return nil, fmt.Errorf("aggchainProverFlow - error getting last sent certificate: %w", err)
}

if lastSentCertificateInfo != nil && lastSentCertificateInfo.Status == agglayer.InError {
a.log.Infof("resending the same InError certificate: %s", lastSentCertificateInfo.String())

bridges, claims, err := a.getBridgesAndClaims(ctx, lastSentCertificateInfo.FromBlock, lastSentCertificateInfo.ToBlock)
if err != nil {
return nil, err
return nil, fmt.Errorf("aggchainProverFlow - error getting bridges and claims: %w", err)
}

if len(bridges) == 0 {
// this should never happen, if it does, we need to investigate
// (maybe someone deleted the bridge syncer db, so we might need to wait for it to catch up)
// just keep return an error here
return nil, fmt.Errorf("we have an InError certificate: %s, but no bridges to resend the same certificate",
return nil, fmt.Errorf("aggchainProverFlow - we have an InError certificate: %s, but no bridges to resend the same certificate",
lastSentCertificateInfo.String())
}

if lastSentCertificateInfo.AggchainProof == "" {
aggchainProof, err := a.aggchainProofClient.GenerateAggchainProof(lastSentCertificateInfo.FromBlock,
lastSentCertificateInfo.ToBlock)
if err != nil {
return nil, fmt.Errorf("error fetching aggchain proof for block range %d : %d : %w",
return nil, fmt.Errorf("aggchainProverFlow - error fetching aggchain proof for block range %d : %d : %w",
lastSentCertificateInfo.FromBlock, lastSentCertificateInfo.ToBlock, err)
}

a.log.Infof("InError certificate did not have auth proof, "+
a.log.Infof("aggchainProverFlow - InError certificate did not have auth proof, "+
"so got it from the aggchain prover for range %d : %d. Proof: %s",
lastSentCertificateInfo.FromBlock, lastSentCertificateInfo.ToBlock, aggchainProof.Proof)

@@ -90,14 +90,14 @@ func (a *aggchainProverFlow) GetCertificateBuildParams(ctx context.Context) (*ty
}

// use the old logic, where we build the new certificate
buildParams, err := a.flowManager.GetCertificateBuildParams(ctx)
buildParams, err := a.baseFlow.GetCertificateBuildParams(ctx)
if err != nil {
return nil, err
}

aggchainProof, err := a.aggchainProofClient.GenerateAggchainProof(buildParams.FromBlock, buildParams.ToBlock)
if err != nil {
return nil, fmt.Errorf("error fetching aggchain proof for block range %d : %d : %w",
return nil, fmt.Errorf("aggchainProverFlow - error fetching aggchain proof for block range %d : %d : %w",
buildParams.FromBlock, buildParams.ToBlock, err)
}

Loading

0 comments on commit 91a3f7e

Please sign in to comment.