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

Problem: sdk 0.50 is not used #407

Merged
merged 1 commit into from
Mar 20, 2024
Merged

Conversation

yihuang
Copy link
Collaborator

@yihuang yihuang commented Feb 22, 2024

Closes: #XXX

Description


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@yihuang yihuang requested a review from mmsqe February 22, 2024 03:51
Copy link
Collaborator

@mmsqe mmsqe left a comment

Choose a reason for hiding this comment

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

do we need regen protos with cosmossdk.io for customtype?

@yihuang
Copy link
Collaborator Author

yihuang commented Feb 22, 2024

do we need regen protos with cosmossdk.io for customtype?

yes, still downloading docker images.

if err != nil {
return nil, err
if len(portPool) == 0 {
return nil, fmt.Errorf("failed to get port for API server")

Check failure

Code scanning / Semgrep OSS

Semgrep Finding: trailofbits.go.missing-unlock-before-return.missing-unlock-before-return Error test

Missing mutex unlock before returning from a function. This could result in panics resulting from double lock operations
if err != nil {
return nil, err
if len(portPool) == 0 {
return nil, fmt.Errorf("failed to get port for RPC server")

Check failure

Code scanning / Semgrep OSS

Semgrep Finding: trailofbits.go.missing-unlock-before-return.missing-unlock-before-return Error test

Missing mutex unlock before returning from a function. This could result in panics resulting from double lock operations
if err != nil {
return nil, err
if len(portPool) == 0 {
return nil, fmt.Errorf("failed to get port for GRPC server")

Check failure

Code scanning / Semgrep OSS

Semgrep Finding: trailofbits.go.missing-unlock-before-return.missing-unlock-before-return Error test

Missing mutex unlock before returning from a function. This could result in panics resulting from double lock operations
if err != nil {
return nil, err
if len(portPool) == 0 {
return nil, fmt.Errorf("failed to get port for JSON-RPC server")

Check failure

Code scanning / Semgrep OSS

Semgrep Finding: trailofbits.go.missing-unlock-before-return.missing-unlock-before-return Error test

Missing mutex unlock before returning from a function. This could result in panics resulting from double lock operations
if err != nil {
return nil, err
if len(portPool) == 0 {
return nil, fmt.Errorf("failed to get port for Proxy server")

Check failure

Code scanning / Semgrep OSS

Semgrep Finding: trailofbits.go.missing-unlock-before-return.missing-unlock-before-return Error test

Missing mutex unlock before returning from a function. This could result in panics resulting from double lock operations
if err != nil {
return nil, err
if len(portPool) == 0 {
return nil, fmt.Errorf("failed to get port for Proxy server")

Check failure

Code scanning / Semgrep OSS

Semgrep Finding: trailofbits.go.missing-unlock-before-return.missing-unlock-before-return Error test

Missing mutex unlock before returning from a function. This could result in panics resulting from double lock operations
@yihuang yihuang marked this pull request as ready for review March 7, 2024 04:57
@yihuang yihuang marked this pull request as draft March 7, 2024 04:57
go.mod Outdated Show resolved Hide resolved
Copy link

codecov bot commented Mar 13, 2024

Codecov Report

Attention: Patch coverage is 59.23913% with 225 lines in your changes are missing coverage. Please review.

Project coverage is 62.53%. Comparing base (94fa5fe) to head (5621fc6).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #407      +/-   ##
===========================================
- Coverage    62.85%   62.53%   -0.32%     
===========================================
  Files          130      130              
  Lines        12361    12495     +134     
===========================================
+ Hits          7769     7814      +45     
- Misses        4064     4135      +71     
- Partials       528      546      +18     
Files Coverage Δ
app/ante/ante.go 57.29% <ø> (ø)
app/ante/fee_checker.go 90.81% <100.00%> (ø)
app/ante/handler_options.go 57.54% <ø> (ø)
ethereum/eip712/eip712_legacy.go 60.32% <ø> (ø)
ethereum/eip712/message.go 82.55% <100.00%> (+0.63%) ⬆️
indexer/kv_indexer.go 64.96% <100.00%> (ø)
rpc/backend/account_info.go 82.66% <100.00%> (ø)
rpc/backend/backend.go 78.94% <ø> (ø)
rpc/backend/blocks.go 85.58% <100.00%> (ø)
rpc/backend/chain_info.go 77.45% <100.00%> (ø)
... and 53 more

@@ -92,7 +92,7 @@
return nil, err
}

go s.start(&s.wg, chHeaders, chTx, chLogs)
go s.start(&s.wg, chBlocks, chTx, chLogs)

Check notice

Code scanning / CodeQL

Spawning a Go routine Note

Spawning a Go routine may be a possible source of non-determinism
store := k.storeService.OpenKVStore(ctx)
bz, err := store.Get(types.ParamsKey)
if err != nil {
panic(err)

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

Possible panics in BeginBock- or EndBlock-related consensus methods could cause a chain halt
func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {
am.keeper.BeginBlock(ctx, req)
func (am AppModule) BeginBlock(ctx context.Context) error {
return am.keeper.BeginBlock(sdk.UnwrapSDKContext(ctx))

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
func (am AppModule) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) {
am.keeper.BeginBlock(ctx, req)
func (am AppModule) BeginBlock(ctx context.Context) error {
return am.keeper.BeginBlock(sdk.UnwrapSDKContext(ctx))

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

path flow from Begin/EndBlock to a panic call
path flow from Begin/EndBlock to a panic call
am.keeper.EndBlock(ctx, req)
return []abci.ValidatorUpdate{}
func (am AppModule) EndBlock(ctx context.Context) error {
return am.keeper.EndBlock(sdk.UnwrapSDKContext(ctx))

Check warning

Code scanning / CodeQL

Panic in BeginBock or EndBlock consensus methods Warning

path flow from Begin/EndBlock to a panic call
Comment on lines +938 to +959
for _, m := range app.ModuleManager.Modules {
if moduleWithName, ok := m.(module.HasName); ok {
moduleName := moduleWithName.Name()
if appModule, ok := moduleWithName.(appmodule.AppModule); ok {
modules[moduleName] = appModule
}
}
}

Check warning

Code scanning / CodeQL

Iteration over map Warning

Iteration over map may be a possible source of non-determinism
go.mod Outdated Show resolved Hide resolved
@mmsqe mmsqe marked this pull request as ready for review March 15, 2024 07:35
x/evm/keeper/keeper.go Outdated Show resolved Hide resolved
fix dependencies

fix build

upgrade protogen image

Co-authored-by: mmsqe <mavis@crypto.com>

wait for block properly

fix py-lint

fix test

fix py-lint
@yihuang yihuang merged commit 00b9e0e into crypto-org-chain:develop Mar 20, 2024
33 of 39 checks passed
@yihuang yihuang deleted the sdk50 branch March 20, 2024 09:46
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.

2 participants