Skip to content

Jg/semgrep 4 #2738

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.

Sign up for GitHub

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

Closed
trianglesphere wants to merge 5 commits into from
Closed

Jg/semgrep 4 #2738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e027a0a
Fix semgrep ignore file
trianglesphere Jun 9, 2022
a48e676
updates
trianglesphere Jun 9, 2022
f7e1278
More resource
trianglesphere Jun 9, 2022
077069e
more updates
trianglesphere Jun 9, 2022
7f3f273
Revert "Use ticker instead of time.After in select loop (#2621)"
trianglesphere Jun 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -508,18 +508,19 @@ jobs:
SEMGREP_BASELINE_REF: << parameters.diff_branch >>
SEMGREP_REPO_URL: << pipeline.project.git_url >>
SEMGREP_BRANCH: << pipeline.git.branch >>
SEMGREP_COMMIT: << pipeline.git.revision >>

# Change job timeout (default is 1800 seconds; set to 0 to disable)
SEMGREP_TIMEOUT: 3000

docker:
- image: returntocorp/semgrep
resource_class: xlarge
steps:
- checkout
- run:
name: "Set environment variables" # for PR comments and in-app hyperlinks to findings
command: |
echo 'export SEMGREP_COMMIT=$CIRCLE_SHA1' >> $BASH_ENV
echo 'export SEMGREP_PR_ID=${CIRCLE_PULL_REQUEST##*/}' >> $BASH_ENV
echo 'export SEMGREP_JOB_URL=$CIRCLE_BUILD_URL' >> $BASH_ENV
echo 'export SEMGREP_REPO_NAME=$CIRCLE_PROJECT_USERNAME/$CIRCLE_PROJECT_REPONAME' >> $BASH_ENV
Expand Down
2 changes: 1 addition & 1 deletion .semgrepignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ tests/

l2geth/
packages/*/node_modules
packages/*/test
packages/*/test
4 changes: 1 addition & 3 deletions op-e2e/geth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ import (

func waitForTransaction(hash common.Hash, client *ethclient.Client, timeout time.Duration) (*types.Receipt, error) {
timeoutCh := time.After(timeout)
ticker := time.NewTicker(100 * time.Millisecond)
defer ticker.Stop()
ctx, cancel := context.WithTimeout(context.Background(), timeout)
defer cancel()
for {
Expand All @@ -42,7 +40,7 @@ func waitForTransaction(hash common.Hash, client *ethclient.Client, timeout time
select {
case <-timeoutCh:
return nil, errors.New("timeout")
case <-ticker.C:
case <-time.After(100 * time.Millisecond):
}
}
Copy link
Contributor

@semgrep-app semgrep-app bot Jun 9, 2022

Choose a reason for hiding this comment

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

Leaky use of time.After in for-select (Lines 32-45)

🔴 Fix or ignore this finding to merge your pull request.
🙈 From dgryski.semgrep-go.timeafter.leaky-time-after.

}
Expand Down
4 changes: 1 addition & 3 deletions op-proposer/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,10 @@ func (s *Service) eventLoop() {
defer s.wg.Done()

name := s.cfg.Driver.Name()
ticker := time.NewTicker(s.cfg.PollInterval)
defer ticker.Stop()

for {
select {
case <-ticker.C:
case <-time.After(s.cfg.PollInterval):
// Determine the range of L2 blocks that the submitter has not
Copy link
Contributor

@semgrep-app semgrep-app bot Jun 9, 2022

Choose a reason for hiding this comment

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

Leaky use of time.After in for-select (Lines 105-171)

🔴 Fix or ignore this finding to merge your pull request.
🙈 From dgryski.semgrep-go.timeafter.leaky-time-after.

// processed, and needs to take action on.
s.l.Info(name + " fetching current block range")
Expand Down
5 changes: 1 addition & 4 deletions op-proposer/txmgr/txmgr.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,15 +205,12 @@ func (m *SimpleTxManager) Send(
wg.Add(1)
go sendTxAsync()

ticker := time.NewTicker(m.cfg.ResubmissionTimeout)
defer ticker.Stop()

for {
select {

// Whenever a resubmission timeout has elapsed, bump the gas
// price and publish a new transaction.
case <-ticker.C:
case <-time.After(m.cfg.ResubmissionTimeout):
// Avoid republishing if we are waiting for confirmation on an
Copy link
Contributor

@semgrep-app semgrep-app bot Jun 9, 2022

Choose a reason for hiding this comment

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

Leaky use of time.After in for-select (Lines 208-235)

🔴 Fix or ignore this finding to merge your pull request.
🙈 From dgryski.semgrep-go.timeafter.leaky-time-after.

// existing tx. This is primarily an optimization to reduce the
// number of API calls we make, but also reduces the chances of
Expand Down