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

Vision: Random call generator for fuzz-testing #251

Open
kianenigma opened this issue Sep 12, 2022 · 11 comments
Open

Vision: Random call generator for fuzz-testing #251

kianenigma opened this issue Sep 12, 2022 · 11 comments
Labels
I5-enhancement An additional feature request. T1-FRAME This PR/Issue is related to core FRAME, the framework.

Comments

@kianenigma
Copy link
Contributor

Some notes about the fuzzing outcome: As you see in the code, I randomly generate calls that may or may not be valid and bombard the pallet with these calls, and every so often I execute the entire set of sanity checks that are defined in the pallet.

We could create a tool which generates a set of calls from the metadata of a pallet. Could even be done in PolkadotJS, as general fuzzy testing framework.

Originally posted by @ggwpez in paritytech/substrate#12002 (comment)

@kianenigma
Copy link
Contributor Author

another alternative would be to use https://docs.rs/arbitrary/latest/arbitrary/. If this works out, it plugs nicely into the rest of the Rust ecosystem.

@ggwpez
Copy link
Member

ggwpez commented Sep 12, 2022

Yea, using a fuzzy library with test-case minimization, auto retry and all the extras would be smart.

@shawntabrizi
Copy link
Member

The problem is that a lot of the pallet calls only make sense when set up with other calls. For example, the steps of a treasury bounty or democracy proposal.

Additionally, i question whether a spectrum of inputs would reveal anything that just looking at the extremes would not reveal. But yeah, if it is easy to implement, would be interested to see. I think SR Labs already does this quite a bit for us when they audit code.

@ggwpez
Copy link
Member

ggwpez commented Sep 12, 2022

The problem is that a lot of the pallet calls only make sense when set up with other calls. For example, the steps of a treasury bounty or democracy proposal.

I think @kianenigma exclusively focused on sanity checks.
But yea, we need some kind of setup and a set of invariants which are then checked each time.

Additionally, i question whether a spectrum of inputs would reveal anything that just looking at the extremes would not reveal. But yeah, if it is easy to implement, would be interested to see. I think SR Labs already does this quite a bit for us when they audit code.

Probably makes the most sense when it interacts with multiple pallets.
Stuff like reference counting, or reserved/locked balance, I think there are a lot of spots where stuff can go wrong, especially when used from different pallets.

@hieronx
Copy link

hieronx commented Sep 12, 2022

I think this is similar to invariant testing as implemented in Foundry, for Solidity: foundry-rs/foundry#1572 (which was inspired by dapptools: https://github.com/dapphub/dapptools/blob/master/src/dapp/README.md#invariant-testing).

I’ve also discussed this a bit with SR Labs recently: they currently are indeed fuzzing calls and checking basic invariants (e.g. “does not panic”), but this can probably be made significantly more effective by:

  • Making the fuzzing more specific by encoding specific setup calls and target methods.
  • Adding invariants specific to a particular pallet rather than just checking for panics.

I definitely agree that this is most useful when interacting with multiple pallets. I've been playing around with the idea of using https://github.com/centrifuge/fudge to a setup a chain, specify a list of calls to make, including passing time (since a lot of our logic depends on time, e.g. debt accrual), and then using a fuzzer (or maybe https://github.com/srlabs/ziggy to be able to use different fuzzers) to generate different states and try to break any of the specified invariants.

As a side note: this is an especially interesting read about the kinds of issues that this can find: https://hackmd.io/@SaferMaker/DAICertoraSurprise. This in particular used Certora's FV toolkit, but the same could probably be found using fuzzing with invariant testing.

@bkchr
Copy link
Member

bkchr commented Sep 12, 2022

If we would use some learning/AI based fuzzer (not sure what the correct naming is here), could we not use this to let it start with random bytes? Start from zero it let it walk through creating and applying transactions. Not sure there would be any useful output coming from this. However, we could try and see :D

@kianenigma
Copy link
Contributor Author

kianenigma commented Sep 13, 2022

The problem is that a lot of the pallet calls only make sense when set up with other calls. For example, the steps of a treasury bounty or democracy proposal.

I don't think this is a showstopper. If you generate enough random calls, with enough iteration, any setup that you can think of should be created. Perhaps you need to call on_inititalize in between as well. I do a similar thing for the pools fuzz testing merged recently, and it does eventually lead to all possible outcomes (at least based on events, I can say we hit a lot of the corners).

Additionally, i question whether a spectrum of inputs would reveal anything that just looking at the extremes would not reveal.

I think this flies in the face of why fuzzing is generally a thing in comp. engineering. The argument is that a lot of edge cases will actually be missed by a human, but a fuzzer can catch. Hard to tell, but I am a fan of not relying on human detected edge cases only.

We can also apply the same logic to the whole runtime and apply random RuntimeCalls.

Indeed, all of this makes little sense if you blindly send these calls. It makes a lot more sense if each pallet would have a set of clear "invariants" (what should be coded as fn try_state henceforth). Then, it is a pretty good testing scenario: regardless of what garbage call you submit, the invariants must be met.

@kianenigma
Copy link
Contributor Author

Adding invariants specific to a particular pallet rather than just checking for panics.

I hope we standardize the try_state or something similar soon, and do actual state testing. Checking for panics only is, in my opinion, even worse, because it actually gives the false sense that things are being checked, but in reality are, in my opinion, very meaningless. A runtime can be extremely faulty but not generate any panics.

@Polkadot-Forum
Copy link

This issue has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/testing-complext-frame-pallets-discussion-tools/356/1

@kianenigma kianenigma changed the title Idea: Random call generator for fuzz-testing Vision: Random call generator for fuzz-testing Mar 10, 2023
@juangirini juangirini transferred this issue from paritytech/substrate Aug 24, 2023
@the-right-joyce the-right-joyce added I5-enhancement An additional feature request. T1-FRAME This PR/Issue is related to core FRAME, the framework. and removed J0-enhancement labels Aug 25, 2023
@louismerlin
Copy link
Contributor

Here is the code SRLabs has been using for the past couple of years to fuzz-test substrate runtimes.

Parity is already aware, but posting here for reference.

https://github.com/srlabs/substrate-runtime-fuzzer

@kianenigma
Copy link
Contributor Author

@louismerlin how fast can this fuzzer reach calls that are decode-able?

For example, if you run the fuzzer for 1 day, what percentage of the inputs fail in actual dispatch and not in decoding?

If this is fast enough, it is good news.

github-merge-queue bot pushed a commit that referenced this issue Nov 5, 2024
This PR updates litep2p to the latest release.

- `KademliaEvent::PutRecordSucess` is renamed to fix word typo
- `KademliaEvent::GetProvidersSuccess` and
`KademliaEvent::IncomingProvider` are needed for bootnodes on DHT work
and will be utilized later


### Added

- kad: Providers part 8: unit, e2e, and `libp2p` conformance tests
([#258](paritytech/litep2p#258))
- kad: Providers part 7: better types and public API, public addresses &
known providers ([#246](paritytech/litep2p#246))
- kad: Providers part 6: stop providing
([#245](paritytech/litep2p#245))
- kad: Providers part 5: `GET_PROVIDERS` query
([#236](paritytech/litep2p#236))
- kad: Providers part 4: refresh local providers
([#235](paritytech/litep2p#235))
- kad: Providers part 3: publish provider records (start providing)
([#234](paritytech/litep2p#234))

### Changed

- transport_service: Improve connection stability by downgrading
connections on substream inactivity
([#260](paritytech/litep2p#260))
- transport: Abort canceled dial attempts for TCP, WebSocket and Quic
([#255](paritytech/litep2p#255))
- kad/executor: Add timeout for writting frames
([#277](paritytech/litep2p#277))
- kad: Avoid cloning the `KademliaMessage` and use reference for
`RoutingTable::closest`
([#233](paritytech/litep2p#233))
- peer_state: Robust state machine transitions
([#251](paritytech/litep2p#251))
- address_store: Improve address tracking and add eviction algorithm
([#250](paritytech/litep2p#250))
- kad: Remove unused serde cfg
([#262](paritytech/litep2p#262))
- req-resp: Refactor to move functionality to dedicated methods
([#244](paritytech/litep2p#244))
- transport_service: Improve logs and move code from tokio::select macro
([#254](paritytech/litep2p#254))

### Fixed

- tcp/websocket/quic: Fix cancel memory leak
([#272](paritytech/litep2p#272))
- transport: Fix pending dials memory leak
([#271](paritytech/litep2p#271))
- ping: Fix memory leak of unremoved `pending_opens`
([#274](paritytech/litep2p#274))
- identify: Fix memory leak of unused `pending_opens`
([#273](paritytech/litep2p#273))
- kad: Fix not retrieving local records
([#221](paritytech/litep2p#221))

See release changelog for more details:
https://github.com/paritytech/litep2p/releases/tag/v0.8.0

cc @paritytech/networking

---------

Signed-off-by: Alexandru Vasile <[email protected]>
Co-authored-by: Dmitry Markin <[email protected]>
lexnv added a commit that referenced this issue Nov 15, 2024
This PR updates litep2p to the latest release.

- `KademliaEvent::PutRecordSucess` is renamed to fix word typo
- `KademliaEvent::GetProvidersSuccess` and
`KademliaEvent::IncomingProvider` are needed for bootnodes on DHT work
and will be utilized later

- kad: Providers part 8: unit, e2e, and `libp2p` conformance tests
([#258](paritytech/litep2p#258))
- kad: Providers part 7: better types and public API, public addresses &
known providers ([#246](paritytech/litep2p#246))
- kad: Providers part 6: stop providing
([#245](paritytech/litep2p#245))
- kad: Providers part 5: `GET_PROVIDERS` query
([#236](paritytech/litep2p#236))
- kad: Providers part 4: refresh local providers
([#235](paritytech/litep2p#235))
- kad: Providers part 3: publish provider records (start providing)
([#234](paritytech/litep2p#234))

- transport_service: Improve connection stability by downgrading
connections on substream inactivity
([#260](paritytech/litep2p#260))
- transport: Abort canceled dial attempts for TCP, WebSocket and Quic
([#255](paritytech/litep2p#255))
- kad/executor: Add timeout for writting frames
([#277](paritytech/litep2p#277))
- kad: Avoid cloning the `KademliaMessage` and use reference for
`RoutingTable::closest`
([#233](paritytech/litep2p#233))
- peer_state: Robust state machine transitions
([#251](paritytech/litep2p#251))
- address_store: Improve address tracking and add eviction algorithm
([#250](paritytech/litep2p#250))
- kad: Remove unused serde cfg
([#262](paritytech/litep2p#262))
- req-resp: Refactor to move functionality to dedicated methods
([#244](paritytech/litep2p#244))
- transport_service: Improve logs and move code from tokio::select macro
([#254](paritytech/litep2p#254))

- tcp/websocket/quic: Fix cancel memory leak
([#272](paritytech/litep2p#272))
- transport: Fix pending dials memory leak
([#271](paritytech/litep2p#271))
- ping: Fix memory leak of unremoved `pending_opens`
([#274](paritytech/litep2p#274))
- identify: Fix memory leak of unused `pending_opens`
([#273](paritytech/litep2p#273))
- kad: Fix not retrieving local records
([#221](paritytech/litep2p#221))

See release changelog for more details:
https://github.com/paritytech/litep2p/releases/tag/v0.8.0

cc @paritytech/networking

---------

Signed-off-by: Alexandru Vasile <[email protected]>
Co-authored-by: Dmitry Markin <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
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. T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
Status: Backlog
Development

No branches or pull requests

9 participants