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

update sims operations to use baseapp #4946

Merged
merged 84 commits into from
Oct 23, 2019
Merged

Conversation

fedekunze
Copy link
Collaborator

@fedekunze fedekunze commented Aug 22, 2019

closes #4935
closes #2778

NOTE FOR REVIEWER: please review with extreme detail. These changes affect all the simulation operations (randomized msgs) which are our best tools for identifying bugs on the code.

  • 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.

  • Wrote tests

  • Updated relevant documentation (docs/)

  • 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, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@fedekunze fedekunze added the WIP label Aug 22, 2019
@fedekunze fedekunze changed the title update operations to use baseapp update sims operations to use baseapp Aug 22, 2019
x/gov/types/expected_keepers.go Outdated Show resolved Hide resolved
x/bank/simulation/operations/msgs.go Outdated Show resolved Hide resolved
x/bank/simulation/operations/msgs.go Outdated Show resolved Hide resolved
x/gov/simulation/operations/msgs.go Outdated Show resolved Hide resolved
x/gov/simulation/operations/msgs.go Outdated Show resolved Hide resolved
x/distribution/simulation/operations/msgs.go Outdated Show resolved Hide resolved
x/slashing/simulation/operations/msgs.go Outdated Show resolved Hide resolved
x/slashing/simulation/operations/msgs.go Outdated Show resolved Hide resolved
x/staking/simulation/operations/msgs.go Outdated Show resolved Hide resolved
x/staking/simulation/operations/msgs.go Outdated Show resolved Hide resolved
x/bank/simulation/operations/msgs.go Outdated Show resolved Hide resolved
x/bank/simulation/operations/msgs.go Outdated Show resolved Hide resolved
x/gov/simulation/operations/msgs.go Outdated Show resolved Hide resolved
)

// GenTx generates a signed mock transaction.
func GenTx(msgs []sdk.Msg, accnums []uint64, seq []uint64, priv ...crypto.PrivKey) auth.StdTx {
Copy link
Collaborator Author

@fedekunze fedekunze Aug 28, 2019

Choose a reason for hiding this comment

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

ideally, we should move all the contents from simapp/test_helpers.go to this file and update the keepers that use it, although that's out of scope from this PR. cc: @colin-axner

@codecov
Copy link

codecov bot commented Aug 28, 2019

Codecov Report

Merging #4946 into master will decrease coverage by 0.01%.
The diff coverage is 4.34%.

@@            Coverage Diff             @@
##           master    #4946      +/-   ##
==========================================
- Coverage   53.55%   53.54%   -0.02%     
==========================================
  Files         290      290              
  Lines       17719    17719              
==========================================
- Hits         9489     9487       -2     
- Misses       7489     7491       +2     
  Partials      741      741

@fedekunze fedekunze self-assigned this Oct 1, 2019
}

if !validator.IsJailed() {
// TODO: due to this condition this message is almost, if not always, skipped !
Copy link
Contributor

Choose a reason for hiding this comment

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

What are we to do with this TODO?

Copy link
Collaborator Author

@fedekunze fedekunze Oct 2, 2019

Choose a reason for hiding this comment

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

actually nothing on this PR. None of the validators is jailed. Idk if that's a problem of the simulator itself

Copy link
Contributor

Choose a reason for hiding this comment

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

we should simulate that by having the undelgate operation sometimes have a validator undelegate below its min delegation causing it to be jailed.

x/slashing/simulation/operations.go Outdated Show resolved Hide resolved
x/bank/simulation/operations.go Outdated Show resolved Hide resolved
x/staking/simulation/operations.go Outdated Show resolved Hide resolved
x/staking/simulation/operations.go Outdated Show resolved Hide resolved
@AdityaSripal AdityaSripal self-assigned this Oct 16, 2019
// returns random subset of the provided coins
// will return at least one coin unless coins argument is empty or malformed
// i.e. 0 amt in coins
func RandSubsetCoins(r *rand.Rand, coins sdk.Coins) sdk.Coins {
Copy link
Member

Choose a reason for hiding this comment

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

I want to use this for RandomFees as well so fees can be paid in multiple coins rather than just one denom. Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I think that's fine.

x/simulation/account.go Outdated Show resolved Hide resolved
x/slashing/simulation/operations.go Outdated Show resolved Hide resolved
x/slashing/simulation/operations.go Outdated Show resolved Hide resolved
x/slashing/simulation/operations.go Outdated Show resolved Hide resolved
simapp/state.go Outdated Show resolved Hide resolved
func GenTxSigLimit(r *rand.Rand) uint64 {
return uint64(r.Intn(7) + 1)
return uint64(r.Intn(7) + 5)
Copy link
Contributor

Choose a reason for hiding this comment

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

irc, the +1 is to just make sure it's never 0. Why not r.Intn(12)+1?

Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

ACK 🎉

x/simulation/account.go Outdated Show resolved Hide resolved
x/slashing/simulation/operations.go Outdated Show resolved Hide resolved
x/slashing/simulation/operations.go Outdated Show resolved Hide resolved
x/slashing/simulation/operations.go Outdated Show resolved Hide resolved
Copy link
Collaborator Author

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

tested ACK

@fedekunze fedekunze merged commit 8344c0a into master Oct 23, 2019
@alexanderbez alexanderbez deleted the fedekunze/4935-sim-ops-baseapp branch November 4, 2019 18:19
larry0x pushed a commit to larry0x/cosmos-sdk that referenced this pull request May 22, 2023
* update operations to use baseapp

* updates and cleanup operations

* update operations

* restructure sim ops params

* rename sim /operations/msg.go to /operations.go

* move GenTx to a helper pkg to avoid circle deps

* rm msg.ValidateBasic

* changelog

* random fees; delete auth's DeductFees sim operation

* add chain-id for sig verification

* Update x/simulation/account.go

Co-Authored-By: colin axner <colinaxner@berkeley.edu>

* fix bank, gov and distr errors

* fix staking and slashing errors; increase prob for send enabled

* increase gas x10

* make format

* fix some distr and staking edge cases

* fix all edge cases

* golang ci

* rename acc vars; default no fees to 0stake

* cleanup; check for exchange rate and skip invalid ops

* fixes

* check for max entries

* add pubkey to genaccounts

* fix gov bug

* update staking sim ops

* fix small redelegation error

* fix small self delegation on unjail

* rm inf loop on random val/accs

* copy array

* add ok boolean to RandomValidator return values

* format

* Update x/bank/simulation/operations.go

Co-Authored-By: colin axner <colinaxner@berkeley.edu>

* Update simapp/helpers/test_helpers.go

Co-Authored-By: colin axner <colinaxner@berkeley.edu>

* address @colin-axner comments

* add genaccount pubkey validation

* fix test

* update operations and move RandomFees to x/simulation

* update gov ops

* address @alexanderbez comments

* avoid modifications to config

* reorder params

* changelog

* Update x/distribution/simulation/genesis.go

Co-Authored-By: Alexander Bezobchuk <alexanderbez@users.noreply.github.com>

* remove named return values

* ensure all operations are simulated

* golangci

* add nolint

* disable whitespace and funlen linter

* disable godox

* add TODO on unjail

* update ops weights

* remove dup

* update godoc

* x/slashing/simulation/operations.go linting

* x/staking/simulation/operations.go linting

* update operations format

* x/bank/simulation/operations.go linting

* x/distribution/simulation/operations.go linting

* x/staking/simulation/operations.go linting

* start changes: make bank simulate send multiple coins, code cleanup

* fix nondeterminism bug

* fix txsiglimit err

* fix multisend bug

* simplify simulation, cleanup opt privkey args

* make slashing test invalid unjail msgs

* Update simapp/state.go

* golangCI changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:baseapp C:Simulations T: API Breaking Breaking changes that impact APIs and the SDK only (not state machine).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simulation: Proper Operation (Message) Execution Simulation: simulate ABCIApp state machine
9 participants