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

Staking Fuzzer to test nomination pool commission & absent calls #424

Open
rossbulat opened this issue Mar 13, 2023 · 9 comments
Open

Staking Fuzzer to test nomination pool commission & absent calls #424

rossbulat opened this issue Mar 13, 2023 · 9 comments
Labels
I5-enhancement An additional feature request.

Comments

@rossbulat
Copy link

rossbulat commented Mar 13, 2023

Motivation

Full coverage of fuzzer testing.

Request

@kianenigma: fuzzer code might need some extra change to add an "agent" that behaves like a pool operator with commission, who will change their commission a bunch of times, and we will compute on the side how much we will think they should earn in sum commission, and then we will check it.

Enhances testing for paritytech/substrate#13128 and paritytech/substrate#12608.

Solution

No response

Are you willing to help with this request?

Yes!

@kianenigma
Copy link
Contributor

@rossbulat I do think this is something that we should not omit for later, ideally we should have the code for this and the fuzzer ready, before this feature goes to Polkadot.

@ggwpez
Copy link
Member

ggwpez commented Apr 1, 2023

Not sure if the staking fuzzers are already integrated into the CI, otherwise we have to do something similar to paritytech/substrate#13673

@rossbulat
Copy link
Author

rossbulat commented Apr 3, 2023

for nomination-pools/fuzzer:

Posting some notes to go by for implementing these changes:

  • Add set_claim_permission into random_call.
  • Add bond_extra_other into random_call.

For an operator specific random loop:

  • Make OPERATOR_AGENT_ACCOUNT and factor it into random_seed_origin to ensure it is not selected.

  • Every X eras:

    • Randomly make operator root of some pool
    • set_commission (if none)
    • set_commission_max (if none)
  • Every X * 64 eras:

    • claim_commission on one of its pools (test actual vs outcome here)

Although not an exhaustive set of scenarios (change rate, setting more restrictive commissions / max commissions) I think the above steps will bootstrap the fuzzer with basic commission testing and aid in future iterations where we can expand to cover all scenarios.

@ggwpez
Copy link
Member

ggwpez commented Apr 3, 2023

The fuzzer fails on master: cd frame/nomination-pools/fuzzer && cargo hfuzz run call with: internal error: entered unreachable code.

@rossbulat
Copy link
Author

rossbulat commented Apr 5, 2023

For me running cargo hfuzz run call does not work on my version of macOS 13.2.1:

  process didn't exit successfully: `/Users/macbookpro/polkadot/substrate/frame/nomination-pools/fuzzer/hfuzz_target/release/build/honggfuzz-249d9f4297928bc9/build-script-build` (exit status: 101)
  --- stderr
  Makefile:103: *** Unsupported MAC OS X version.  Stop.

I assume this can only be tested in a VM / on a server if one is on Apple Silicon.

@ggwpez
Copy link
Member

ggwpez commented Apr 5, 2023

I assume this can only be tested in a VM / on a server if one is on Apple Silicon.

Do you have cargo remote setup? Could help you here.

@kianenigma
Copy link
Contributor

Yes, we should add the existing fuzzer to the CI + make sure the commission feature is integrated into it.

@ggwpez
Copy link
Member

ggwpez commented Apr 21, 2023

We can even do that async and allow it to fail in the CI now.
@altaua are you on-board? Basically the same as paritytech/substrate#13673 but for the nomination-pool fuzzers.
I dont know if there is a smarter way to do this, maybe we can just run the hfuzz in the Substrate root? Or have a list of folders that it needs to run?

@altaua
Copy link
Contributor

altaua commented Jun 27, 2023

I've got a PR ready to enable the nomination pool fuzzer. It still fails on master though, can we get that fixed?

@juangirini juangirini transferred this issue from paritytech/substrate Aug 24, 2023
@the-right-joyce the-right-joyce added I5-enhancement An additional feature request. and removed J0-enhancement labels Aug 25, 2023
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
jonathanudd pushed a commit to jonathanudd/polkadot-sdk that referenced this issue Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I5-enhancement An additional feature request.
Projects
Status: ⌛️ Sometime-soon
Development

No branches or pull requests

5 participants