Alex/cheatcodes initial

[Alexangelj]

Give an overview of the tasks completed
Closes #485 by creating the "Cheatcode" item of the Instruction enum and a "Cheatcodes" enum with the implemented cheatcodes. Adds a method apply_cheatcode to the client to send cheatcode instructions to the enviroment, very straightforward stuff!

Link to issue(s) that this PR closes
#485

Changes

• Removes Instruction::Deal
• Removes Outcome::DealCompleted
• Removes Middleware::deal()
• Adds Instruction::Cheatcode
• Adds Cheatcodes enum
• Adds Middleware::apply_cheatcode()
• Implements the Store, Load, and Deal cheatcodes in the environment instruction processing switch
• Adds a test for the store and load cheatcodes
• Edits the deal tests to use the new apply_cheatcode method

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (feat(arbiter-core)/v0.6.0@b16b6ef). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head bba2029 differs from pull request most recent head ca20f32. Consider uploading reports for the commit ca20f32 to get more accurate results

@@                     Coverage Diff                      @@
##             feat(arbiter-core)/v0.6.0     #523   +/-   ##
============================================================
  Coverage                             ?   59.00%           
============================================================
  Files                                ?       11           
  Lines                                ?     3827           
  Branches                             ?        0           
============================================================
  Hits                                 ?     2258           
  Misses                               ?     1569           
  Partials                             ?        0           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[Alexangelj]

It's not quite ready - Load cheatcode doesn't work because apply_cheatcode method does not return a value, which some cheatcodes might. Will need to think about how to handle this, since the return values could be non-existent or a certain type.

[0xJepsen]

It's not quite ready - Load cheatcode doesn't work because apply_cheatcode method does not return a value, which some cheatcodes might. Will need to think about how to handle this, since the return values could be non-existent or a certain type.

Could we have the apply_cheatcode return an option? Or does it need to have it's signature changed at all since it sends result to the outcomesender?

[0xJepsen]

Asked some questions

[Alexangelj]

It's not quite ready - Load cheatcode doesn't work because apply_cheatcode method does not return a value, which some cheatcodes might. Will need to think about how to handle this, since the return values could be non-existent or a certain type.

Could we have the apply_cheatcode return an option? Or does it need to have it's signature changed at all since it sends result to the outcomesender?

Yeah I'm leaning towards not changing the signature and just using the outcome sender stuff to get the result - but trying to do that and running into some errors, so fixing that

[Alexangelj]

Figured out the bug - since we are "catching" the results of applying a cheatcode, if we fail to catch one of the apply's we might get the return value of a different cheatcode, e.g.:

cheatcode::store -> dont catch
cheatcode::load -> catch

we catch the store result here, not the load result

edit: it's not the best developer experience to remind everyone they need to catch all the results

[0xJepsen]

if we fail to catch one of the apply's we might get the return value of a different cheatcode, e.g.:

Figured out the bug - since we are "catching" the results of applying a cheatcode, if we fail to catch one of the apply's we might get the return value of a different cheatcode, e.g.:

cheatcode::store -> dont catch cheatcode::load -> catch

we catch the store result here, not the load result

edit: it's not the best developer experience to remind everyone they need to catch all the results

Jus to clarify, are you saying that it's not a great devex to propagate the errors back with results? What do you mean by catch

[Alexangelj]

if we fail to catch one of the apply's we might get the return value of a different cheatcode, e.g.:

Figured out the bug - since we are "catching" the results of applying a cheatcode, if we fail to catch one of the apply's we might get the return value of a different cheatcode, e.g.:
cheatcode::store -> dont catch cheatcode::load -> catch
we catch the store result here, not the load result
edit: it's not the best developer experience to remind everyone they need to catch all the results

Jus to clarify, are you saying that it's not a great devex to propagate the errors back with results? What do you mean by catch

Yeah I mean if you don't catch it, and you try to catch something else later, you might catch the result you didnt catch earlier in the outcome sender recv

[Alexangelj]

I fixed it with my latest commit - for some reason the submodules also got pushed?

[Autoparallel]

I will review this once the merge conflicts are resolved if that is all that's left

[0xJepsen]

I fixed it with my latest commit - for some reason the submodules also got pushed?

Oh man yeah this is a pain, i think @kinrezC has a nice trick to remove them recursively so they don't get pushed? After that, and the merge conflicts it should be good to go

[Alexangelj]

Alright, good to review + merge!

[0xJepsen]

Only thing i have a problem with is the gitsubmodules, can we get those removed?

[Alexangelj]

Only thing i have a problem with is the gitsubmodules, can we get those removed?

Removed!

[0xJepsen]

lgtm! Are we still merging into this feature branch?