feat(cheatcodes): base implementation of cheatcode system by tchataigner · Pull Request #58 · polyphene/kythera

tchataigner · 2023-04-18T18:27:09Z

Description

This PR introduces cheatcodes in Kythera. The inspiration for what the system allows is heavily inspired from Foundry's cheatcodes.

Changelog

Major

We now implement most of the components from the ref-fvm repo as we need to have granular control over them for the cheatcode system to work. This implementation is heavily inspired from the conformance test framework from @Stebalian on the ref-fvm
- KytheraMachine: Stores override values.
- KytheraCallManager: Set override values when detects a send() syscall to actor Id 98 (cheatcodes actor).
- KytheraKernel: Actually override values at call time.
Introducing an OverrideContext structure that allows us to store at the KytheraMachine level different value to alter information when cheatcodes are invoked.
Deploying a Cheatcode Actor at Actor Id 98. It contains entrypoints corresponding to the different cheatcodes we want to be available to our users.
Updated the way we compile actors to get binaries. Now, all actors can be found under <project-root>/actors. In this folder we find both actors (available at all time) and test-actors (only available if feature=["testing"] is enabled). This allows to have some binaries built and used in our kythera-lib without actually building everything everytime.

Minor

Actor are no longer deployed at a random ID. Instead, we first create an address from their new and register it on-chain.
Set default Kythera chain Id to 1312

Links to any relevant issues or pull requests

Closes #22

Change checklist

I have performed a self-review of my own code
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works

* Upgrade builtins dependencies to revision for release 10.0 (M2.1 of FVM)

* Using code_by_id() method to retrieve builtins code CID from Manifest * Get builtins actor ID from builtin crate

from the rest of the methods.

* Removed dependencies to builtin actors to rely on Chainsafe repo instead as per filecoin-project/builtin-actors#194 (comment)

* Setup all necessary builtins in our local development environment

* Adding actor to test builtins deployment in Kythera

* Adding actor to test constructor and setup being called in test flow

* Updating the builtin test actor to test that all builtins code are deployed at their address * Fix clippy

- introduce constructor_setup_test_actor crate to test Setup and Constructor functions

Co-authored-by: PhilippeMts <1371613+PhilippeMts@users.noreply.github.com>

Co-authored-by: João Oliveira <hello@jxs.pt>

…ra into feature/builtins-genesis

…e-constructor-setup

* Needed to have a Cheatcodes actor incorporated in our library so re-used the kythera_test_actors crate logic to generate it. kythera_test_actors have been renamed to kythera_actors. * Created a testing feature for kythera_actors to only build test_actors when we need them (mostly during tests of our kythera_lib).

…r cheatcodes

jxs

Thanks for this Thomas, first Round of review, I still need to read the actors and the handle_cheatcode function

jxs · 2023-04-19T16:06:45Z

+    {
+        Self {
+            inner: K::new(
+                KytheraCallManager(mgr),


This file needs all these overrides mainly because of this right? I.e to be able to abstract the CallManager, can we make a comment stating that with a TODO to fix this upstream?

This is one of the reasons along with overriding of context in msg_context() and network_context(). So I'm not 100% sure that changing the upstream to remove the passing of the call manager will actually have an impact on our need to do what's currently being implemented (as we are actually overriding the logic of some methods).

What's your take on that ?

jxs · 2023-04-19T16:13:33Z

+pub(crate) const PRANK_METHOD: &str = "Prank";
+pub(crate) const TRICK_METHOD: &str = "Trick";
+
+lazy_static::lazy_static! {


nit: OnceCell is going to be stable soon so it probably makes sense to use its crate instead

wdyt of actually calculating them and hardcoding them? We can then have tests for them on this module to assert that they correspond to the strings and then we could match on them with u64's on handle_cheatcode

jxs · 2023-04-19T16:14:49Z

 /// Main interface to test `Actor`s with Kythera.
 pub struct Tester {
-    // Builtin actors root Cid used in the Machine
+    // Builtin test_actors root Cid used in the Machine


are the builtin actors test actors?

No ! Good one, must have been a wrong update on my side. Thanks for that 🙏

Co-authored-by: João Oliveira <hello@jxs.pt>

jxs · 2023-04-20T11:27:53Z

+pub(crate) const PRANK_METHOD: &str = "Prank";
+pub(crate) const TRICK_METHOD: &str = "Trick";
+
+lazy_static::lazy_static! {


wdyt of actually calculating them and hardcoding them? We can then have tests for them on this module to assert that they correspond to the strings and then we could match on them with u64's on handle_cheatcode

Co-authored-by: João Oliveira <hello@jxs.pt>

fix(cheatcodes): typo fix(cheatcodes): typo

…kythera into feature/cheatcodes-foundation

jxs

LGTM Thomas, only two minor comments left.

jxs · 2023-04-21T11:40:14Z

-    /// Deploy a new Actor at a given address, provided with a given token balance
-    /// and returns the CodeCID of the installed actor
-    pub fn deploy_actor_from_bin(
+    fn deploy_actor_from_bin_at_address(


this should still have a comment no?

Co-authored-by: João Oliveira <hello@jxs.pt>

…at_address

tchataigner and others added 30 commits April 3, 2023 14:39

feat(builtins): upgrade builtins dep

939cc75

* Upgrade builtins dependencies to revision for release 10.0 (M2.1 of FVM)

feat(builtins): updated way to retrieve builtin code

a8d80e4

* Using code_by_id() method to retrieve builtins code CID from Manifest * Get builtins actor ID from builtin crate

fix(builtins): bump helix lib & builtins lob

3f92a49

cli: rename read_actor and log the reason it failed to read.

1354bbf

common: separate constructor and set_up functions

f3a3ed3

from the rest of the methods.

lib: execute Constructor and SetUp first

4901439

feat(builtins): integrating builtins deployment

a75839f

feat(builtins): chainsafe builtins dep

4e191ab

* Removed dependencies to builtin actors to rely on Chainsafe repo instead as per filecoin-project/builtin-actors#194 (comment)

feat(builtins): stable channel for rust toolchain

fa10194

feat(builtins): setup all builtins

ce8334d

* Setup all necessary builtins in our local development environment

lib: address Thomas review

e46769c

feat(builtins): builtins tests

5f0f338

* Adding actor to test builtins deployment in Kythera

feat(lib): testing flow

b122876

* Adding actor to test constructor and setup being called in test flow

feat(builtins): testing builtins deployment

ee34622

* Updating the builtin test actor to test that all builtins code are deployed at their address * Fix clippy

fix(builtins): changing Chainsafe dep to be based on crates.io

c0683e8

fix(builtins): merge develop

7cffa99

lib: extract executor into it's own mod

b893074

- introduce constructor_setup_test_actor crate to test Setup and Constructor functions

Update test_actors/actors/builtins_test_actor/src/actor.rs

ab0ebd8

Co-authored-by: PhilippeMts <1371613+PhilippeMts@users.noreply.github.com>

Update lib/tests/lib.rs

84177e2

Co-authored-by: João Oliveira <hello@jxs.pt>

fix(builtins): refactor test code

089d096

fix(builtins): fixed typo in variable

409e736

Merge branch 'develop' into feature/builtins-genesis

86bd74e

Merge branch 'develop' into feature/builtins-genesis

7502afa

Merge branch 'feature/builtins-genesis' of github.com:polyphene/kythe…

1011b1e

…ra into feature/builtins-genesis

lib: address Thomas review

039b80b

Merge branch 'develop' of github.com:polyphene/kythera into feat/pars…

10a7b28

…e-constructor-setup

feat(cheatcodes): started cheatcodes testing ground

97a2639

lib: address Thomas review

8f9063c

fix(cheatcodes): merge feat/parse-constructor-setup

7fa89f1

tchataigner added 4 commits April 11, 2023 12:01

fix(cheatcodes): merge develop

d8f27b6

feat(cheatcodes): WIP trying to implement a persistent context for ou…

1a70fad

…r cheatcodes

feat(cheatcodes): cheatcodes for Kythera

af435a5

fix(cheatcodes): merge develop

2e318ae

tchataigner requested a review from jxs April 18, 2023 18:37

jxs reviewed Apr 19, 2023

View reviewed changes

tchataigner and others added 2 commits April 20, 2023 15:33

Update lib/src/state_tree.rs

309c054

Co-authored-by: João Oliveira <hello@jxs.pt>

Update cli/src/utils/search.rs

45c431a

Co-authored-by: João Oliveira <hello@jxs.pt>

jxs reviewed Apr 20, 2023

View reviewed changes

tchataigner and others added 3 commits April 20, 2023 18:06

Update cli/src/utils/search.rs

e508ce8

Co-authored-by: João Oliveira <hello@jxs.pt>

Update cli/src/utils/search.rs

92f89f8

Co-authored-by: João Oliveira <hello@jxs.pt>

fix(cheatcodes): address jxs review

f8216ce

tchataigner requested a review from jxs April 20, 2023 16:34

tchataigner added 5 commits April 21, 2023 08:55

fix(cheatcodes): jxs review addressed

0661191

fix(cheatcodes): typo

def912e

fix(cheatcodes): typo

baa541d

fix(cheatcodes): typo

4b2866c

fix(cheatcodes): typo fix(cheatcodes): typo

Merge branch 'feature/cheatcodes-foundation' of github.com:polyphene/…

cc43577

…kythera into feature/cheatcodes-foundation

jxs previously approved these changes Apr 21, 2023

View reviewed changes

tchataigner dismissed jxs’s stale review via 0054791 April 21, 2023 11:58

tchataigner and others added 2 commits April 21, 2023 13:58

Update lib/src/state_tree.rs

0054791

Co-authored-by: João Oliveira <hello@jxs.pt>

fix(cheatcodes): typo in comments + comment on deploy_actor_from_bin_…

9c98ad7

…at_address

tchataigner requested a review from jxs April 21, 2023 12:02

fix(cheatcodes): merge develop

2f0a6ef

jxs previously approved these changes Apr 21, 2023

View reviewed changes

fix(cheatcodes): remove unused feature

427afcd

tchataigner dismissed jxs’s stale review via 427afcd April 24, 2023 07:48

tchataigner merged commit de142e0 into develop Apr 24, 2023

tchataigner mentioned this pull request Apr 25, 2023

Testing utilities: assertions in tests #22

Closed

Conversation

tchataigner commented Apr 18, 2023

Description

Changelog

Links to any relevant issues or pull requests

Change checklist

Uh oh!

jxs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jxs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants