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.

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

Jump to bottom

Problem: sim tests are removed #304

Merged
merged 5 commits into from
Sep 7, 2023
Merged

Problem: sim tests are removed #304

merged 5 commits into from
Sep 7, 2023

Conversation

mmsqe
Copy link
Collaborator

@mmsqe mmsqe commented Jul 31, 2023

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)

@mmsqe
Copy link
Collaborator Author

mmsqe commented Jul 31, 2023

/runsim

github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 31, 2023
View reviewed changes
app/utils.go Fixed Show fixed Hide fixed
x/evm/simulation/genesis.go
import (
"encoding/json"
"fmt"
"math/rand"

Check warning

Code scanning / Semgrep

Do not use `math/rand`. Use `crypto/rand` instead.

Do not use `math/rand`. Use `crypto/rand` instead.
x/evm/simulation/operations.go
"encoding/json"
"fmt"
"math/big"
"math/rand"

Check warning

Code scanning / Semgrep

Do not use `math/rand`. Use `crypto/rand` instead.

Do not use `math/rand`. Use `crypto/rand` instead.
x/evm/simulation/params.go Fixed Show fixed Hide fixed
.github/workflows/test.yml
go-version: 1.19
check-latest: true
- uses: actions/checkout@v3
- uses: technote-space/get-diff-action@v6.1.1

Check warning

Code scanning / Semgrep

Semgrep Finding: yaml.github-actions.security.third-party-action-not-pinned-to-commit-sha.third-party-action-not-pinned-to-commit-sha

An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload.
.github/workflows/test.yml
go-version: 1.19
check-latest: true
- uses: actions/checkout@v3
- uses: technote-space/get-diff-action@v6.1.1

Check warning

Code scanning / Semgrep

Semgrep Finding: yaml.github-actions.security.third-party-action-not-pinned-to-commit-sha.third-party-action-not-pinned-to-commit-sha

An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload.
.github/workflows/test.yml
go-version: 1.19
check-latest: true
- uses: actions/checkout@v3
- uses: technote-space/get-diff-action@v6.1.1

Check warning

Code scanning / Semgrep

Semgrep Finding: yaml.github-actions.security.third-party-action-not-pinned-to-commit-sha.third-party-action-not-pinned-to-commit-sha

An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload.
.github/workflows/test.yml
go-version: 1.19
check-latest: true
- uses: actions/checkout@v3
- uses: technote-space/get-diff-action@v6.1.1

Check warning

Code scanning / Semgrep

Semgrep Finding: yaml.github-actions.security.third-party-action-not-pinned-to-commit-sha.third-party-action-not-pinned-to-commit-sha

An action sourced from a third-party repository on GitHub is not pinned to a full length commit SHA. Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload.
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 31, 2023
View reviewed changes
x/evm/simulation/operations.go
// Transferable amount is between the range [0, spendable), spendable = balance - gasFeeCap * GasLimit.
func RandomTransferableAmount(ctx *simulateContext, address common.Address, estimateGas uint64, gasFeeCap *big.Int) (amount *big.Int, err error) {
balance := ctx.keeper.GetBalance(ctx.context, address)
feeLimit := new(big.Int).Mul(gasFeeCap, big.NewInt(int64(estimateGas)))

Check failure

Code scanning / gosec

Potential integer overflow by integer type conversion

Potential integer overflow by integer type conversion
x/evm/simulation/operations.go
return nil, err
}
// we suppose that gasLimit should be larger than estimateGas to ensure tx validity
gasLimit := estimateGas + uint64(ctx.rand.Intn(int(sdktx.MaxGasWanted-estimateGas)))

Check failure

Code scanning / gosec

Potential integer overflow by integer type conversion

Potential integer overflow by integer type conversion
x/evm/simulation/operations.go
return nil, err
}
// we suppose that gasLimit should be larger than estimateGas to ensure tx validity
gasLimit := estimateGas + uint64(ctx.rand.Intn(int(sdktx.MaxGasWanted-estimateGas)))

Check failure

Code scanning / gosec

Potential integer overflow by integer type conversion

Potential integer overflow by integer type conversion
app/utils.go Fixed Show fixed Hide fixed
app/utils.go

prv := secp256k1.GenPrivKeyFromSecret(privkeySeed)
ethPrv := &ethsecp256k1.PrivKey{}
_ = ethPrv.UnmarshalAmino(prv.Bytes()) // UnmarshalAmino simply copies the bytes and assigns them to ethPrv.Key

Check warning

Code scanning / gosec

Returned error is not propagated up the stack.

Returned error is not propagated up the stack.
app/utils.go
for i := 0; i < n; i++ {
// don't need that much entropy for simulation
privkeySeed := make([]byte, 15)
_, _ = r.Read(privkeySeed)

Check warning

Code scanning / gosec

Returned error is not propagated up the stack.

Returned error is not propagated up the stack.
@mmsqe mmsqe marked this pull request as ready for review July 31, 2023 04:37
@mmsqe mmsqe requested a review from yihuang July 31, 2023 04:37
github-advanced-security[bot]
github-advanced-security bot found potential problems Aug 2, 2023
View reviewed changes
app/utils.go
@@ -17,6 +17,7 @@

import (
"encoding/json"
"math/rand"

Check warning

Code scanning / Semgrep

Semgrep Finding: go.lang.security.audit.crypto.math_random.math-random-used

Do not use `math/rand`. Use `crypto/rand` instead.
github-advanced-security[bot]
github-advanced-security bot found potential problems Aug 2, 2023
View reviewed changes
app/utils.go
@@ -17,6 +17,7 @@

import (
"encoding/json"
"math/rand"

Check failure

Code scanning / gosec

Blocklisted import runtime

Blocklisted import math/rand
@mmsqe mmsqe merged commit 3eb3523 into crypto-org-chain:develop Sep 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yihuang yihuang yihuang approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mmsqe @yihuang