Skip to content
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

feat: protect ssender nonce #67

Merged
merged 4 commits into from
Sep 9, 2024
Merged

feat: protect ssender nonce #67

merged 4 commits into from
Sep 9, 2024

Conversation

ToniRamirezM
Copy link
Contributor

@ToniRamirezM ToniRamirezM commented Sep 9, 2024

Description

Protects sequence sender nonce using a mutex, to avoid issues detected in parallel mode.

@ToniRamirezM ToniRamirezM requested a review from a team September 9, 2024 07:34
@ToniRamirezM ToniRamirezM self-assigned this Sep 9, 2024
@ToniRamirezM ToniRamirezM marked this pull request as ready for review September 9, 2024 07:34
@@ -131,6 +132,9 @@ func New(cfg Config, etherman *etherman.Client, txBuilder txbuilder.TxBuilder) (

// Start starts the sequence sender
func (s *SequenceSender) Start(ctx context.Context) {
s.nonceMutex.Lock()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT if we lock only this critical section?

s.currentNonce, err = s.etherman.CurrentNonce(ctx, s.cfg.L2Coinbase)
if err != nil {
    log.Fatalf("failed to get current nonce from %v, error: %v", s.cfg.L2Coinbase, err)
} else {
    log.Infof("current nonce for %v is %d", s.cfg.L2Coinbase, s.currentNonce)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but honestly this is the Start function. It will never be executed in parallel and it is not even needed to protect that variable in this function. But it is ok for me to just protect in that section. I will do the change.

Copy link

sonarqubecloud bot commented Sep 9, 2024

@ToniRamirezM ToniRamirezM merged commit 10fee93 into develop Sep 9, 2024
9 checks passed
@ToniRamirezM ToniRamirezM deleted the feature/protectNonce branch September 9, 2024 14:19
Stefan-Ethernal pushed a commit that referenced this pull request Sep 17, 2024
* feat: protect ssender nonce
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants