Skip to content

plt-228 fixed static check on app and evmrpc package#3154

Merged
amir-deris merged 3 commits intomainfrom
amir/plt-228-fix-staticcheck-app-evmrpc
Apr 1, 2026
Merged

plt-228 fixed static check on app and evmrpc package#3154
amir-deris merged 3 commits intomainfrom
amir/plt-228-fix-staticcheck-app-evmrpc

Conversation

@amir-deris
Copy link
Copy Markdown
Contributor

Describe your changes and provide context

Fixing some of the static check issues reported by staticcheck -tags=codeanalysis ./..., in packages app and evmrpc:

✅ app/app.go:1505:2: field result is unused (U1000)
✅ app/benchmark/contracts.go:86:6: func packConstructorArgs is unused (U1000)
✅ evmrpc/rpcstack.go:361:22: func (*HTTPServer).stopWS is unused (U1000)
✅ evmrpc/stats/tracker.go:233:19: func (*tracker).reportPeriod is unused (U1000)
❌ evmrpc/subscribe.go:254:14: s.tmClient.Subscribe is deprecated: This method will be removed in Tendermint v0.37, use Events instead.  (SA1019)
❌ evmrpc/subscribe.go:268:9: s.tmClient.Unsubscribe is deprecated: This method will be removed in Tendermint v0.37, use Events instead.  (SA1019)
✅evmrpc/subscribe_test.go:71:25: calling regexp.MatchString in a loop has poor performance, consider using regexp.Compile (SA6000)

Did not make changes for evmrpc/subscribe deprecation issue as it would require more refactoring.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 31, 2026

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedApr 1, 2026, 6:15 PM

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 31, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.73%. Comparing base (3ed3bf2) to head (9831292).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3154      +/-   ##
==========================================
- Coverage   58.75%   58.73%   -0.02%     
==========================================
  Files        2095     2095              
  Lines      173551   173484      -67     
==========================================
- Hits       101965   101903      -62     
+ Misses      62465    62464       -1     
+ Partials     9121     9117       -4     
Flag Coverage Δ
sei-chain-pr 58.69% <ø> (?)
sei-db 70.41% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
app/app.go 69.00% <ø> (ø)
app/benchmark/contracts.go 0.00% <ø> (ø)
evmrpc/rpcstack.go 74.91% <ø> (ø)
evmrpc/stats/tracker.go 85.82% <ø> (+2.62%) ⬆️

... and 35 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown
Contributor

@mojtaba-esk mojtaba-esk left a comment

Choose a reason for hiding this comment

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

Great work Amir!
I would add the static check (or equivalent) to the CI so nothing regresses.
Also left some comments, no major blockers.

}

// stopWS disables JSON-RPC over WebSocket and also stops the server if it only serves WebSocket.
func (h *HTTPServer) stopWS() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[nit] stopWs encodes a real lifecycle rule: disable WS, then stop the whole server if HTTP RPC is also off. Today nothing calls it, so removing it is correct, but it is the kinda helper that might be reintroduced when someone wires dynamic enable/disable. Not a blocker; just worth a one-line comment in the PR or a pointer to disableWS / shutdown path so future authors do not duplicate locking incorrectly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for feedback. I will bring this method back with a comment to linter to not show as warning.

return txResults
}

type ChannelResult struct {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Safe to remove if unused, but Giga work is ongoing, so someone might have planned to use it. Worth a quick checked with the team that works on Giga? to be 100% sure about it. or someone from their team to have a look at this line.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I asked in the consensus-eng channel and haven't heard any complaints. Also this code seems to be from 2022 so I doubt it is being used for anything recent.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

#3151
This is the current attempt to plug App into Autobahn, so I don't think we use ChannelResult

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yup, safe to remove

@amir-deris
Copy link
Copy Markdown
Contributor Author

Great work Amir! I would add the static check (or equivalent) to the CI so nothing regresses. Also left some comments, no major blockers.

Thanks for feedback Mojtaba! I will work on the comments you mentioned.

@amir-deris amir-deris enabled auto-merge April 1, 2026 18:30
@amir-deris amir-deris added this pull request to the merge queue Apr 1, 2026
Merged via the queue into main with commit 3d64c1b Apr 1, 2026
39 checks passed
@amir-deris amir-deris deleted the amir/plt-228-fix-staticcheck-app-evmrpc branch April 1, 2026 19:24
yzang2019 added a commit that referenced this pull request Apr 1, 2026
* main:
  plt-228 fixed static check on app and evmrpc package (#3154)
  flatkv cache (#3027)
  Make cryptosim state store backend configurable + No Op Wrapper + Read Disable Config (#3145)
  Add warning message for IAVL deprecation (#3159)
  Change default min valid per window to zero (#3157)
  support for starting autobahn from non-zero global block (#3136)
  Fix upgrade list comparison to respect semver (#3153)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants