Contract mock system #583

taminomara · 2021-07-13T10:13:06Z

Fix #83.

Implement mock system for contracts. Currently mimicking mockall interface.

See the attached issue for more details and discussion.

taminomara · 2021-07-13T10:15:57Z

This is a work in progress PR. I'll be updating it from time to time, although I'm focusing on other issues for now. You're welcome to comment on it (see also #83), but bear in mind that this code is far from its final state, and I expect it be changed quite a lot.

ethcontract/src/mock.rs

taminomara · 2021-07-26T14:23:37Z

This is still WIP, but the version with a simple callback for each contract method works (see test).

fleupold

Looks very promising!

fleupold · 2021-07-28T08:24:36Z

ethcontract-mock/src/details.rs

+
+        state.block += 1;
+
+        let receipt = TransactionReceipt {


I'm not sure if it's a good idea to leave most values empty here. Maybe we should check how a "normal" tx receipt looks like and use default hash values instead of None values?

ethcontract-mock/src/lib.rs

ethcontract-mock/src/parse.rs

fleupold · 2021-07-28T08:39:03Z

ethcontract-mock/src/parse.rs

+    }
+
+    // Unwraps `Result`, adds info with current arg index and rpc name.
+    fn res<T, E: Display>(&self, res: Result<T, E>) -> T {


nit: maybe more descriptive name?

ethcontract-mock/src/test.rs

ethcontract-mock/src/details.rs

taminomara · 2021-07-28T15:20:38Z

I'll add docs and tests later today. The code is ready for review!

nlordell

Finally! Mocked contracts!

As a general comment this PR was very large and quite difficult to review. It would have been nice to have been split into smaller pieces that could be merged (even if they were effectively dead code for a few commits) just to make the review process easier.

nlordell · 2021-07-30T08:34:01Z

ethcontract/src/contract.rs

+/// [`method`]: `Instance::method`
+/// [`view_method`]: `Instance::view_method`
+#[derive(Debug, Copy, Clone, Hash, Eq, PartialEq)]
+pub struct Signature<P, R>(pub H32, pub std::marker::PhantomData<(P, R)>);


nlordell · 2021-07-30T08:36:46Z

ethcontract-mock/src/details/mod.rs

+    receipts: HashMap<H256, TransactionReceipt>,
+}
+
+impl MockTransport {


Wow! You implemented a mock transport that handles all the ethcontract necessary RPC calls!

One thing that I worry about is that we may make use of additional RPC methods in ethcontract and forget to add them here. I can't think of a good way to deal with this though.

nlordell · 2021-07-30T08:38:42Z

ethcontract-mock/src/details/mod.rs

+        // We could support signing if user adds accounts with their private
+        // keys during mock setup.
+
+        panic!("mock node can't sign transactions, use offline signing with private key");


We can just sign with PrivateKey::ONE in these cases.

We have to sign with private key for the from address, unfortunately. Otherwise transaction hash will be botched and something might break.

If from is None we can still use PrivateKey::ONE 😄.

nlordell · 2021-07-30T08:39:51Z

ethcontract-mock/src/details/mod.rs

+                panic!("mock node does not support executing methods on earliest block");
+            }
+            BlockNumber::Number(n) if n.as_u64() != state.block => {
+                panic!("mock node does not support executing methods on non-last block");


This is something we likely need for GPv2 services. However, seeing as the PR is already quite large, we can leave it for later.

In hopes of avoiding this issue for as long as possible we allow specific block number if it's equal to the current top block number. But you're right, we'll have to support this eventually.

ethcontract-mock/src/details/mod.rs

nlordell · 2021-07-30T08:43:04Z

ethcontract-mock/src/details/mod.rs

+        let mut state = self.state.lock().unwrap();
+        let expectation = state.expectation::<P, R>(address, signature, index, generation);
+        expectation.returns = Returns::Default;
+    }


Might also be worth adding a mine function to increment the block number.

Also something for another PR (as this one is already quite large).

nlordell · 2021-07-30T08:45:24Z

ethcontract-mock/src/lib.rs

+        );
+        self
+    }
+}


Very nice documentation.

nlordell · 2021-07-30T08:45:52Z

ethcontract-mock/src/lib.rs

+    /// [`expect`]: Contract::expect
+    pub fn checkpoint(&self) {
+        self.transport.checkpoint();
+    }


similar comment about a mine method.

vkgnosis

Nice.

ethcontract-mock/src/details/mod.rs

ethcontract-mock/src/details/parse.rs

taminomara · 2021-07-30T13:49:22Z

As a general comment this PR was very large and quite difficult to review.

Sorry about that, I should've made smaller PRs throughout the development process.

bh2smith

All of this looks really great although some of it is a bit beyond me to see how so many different things connect. Perhaps smaller incremental changes would have helped focus on some of the more abstract concepts. I will continue to read and review, but it is looking pretty good overall.

ethcontract-mock/src/details/default.rs

taminomara · 2021-08-16T08:29:43Z

I want to merge and release this PR today. I'll merge it without squashing to preserve commit history. I've also made an interactive rebase to account for web3 update that has happened in main.

taminomara requested a review from a team July 13, 2021 10:15

bh2smith reviewed Jul 14, 2021

View reviewed changes

ethcontract/src/mock.rs Outdated Show resolved Hide resolved

taminomara force-pushed the ethcontract-rs-83 branch 3 times, most recently from 1ec8642 to 997ef38 Compare July 26, 2021 14:20

taminomara force-pushed the ethcontract-rs-83 branch from dc6841b to b0dd70b Compare July 26, 2021 14:41

fleupold reviewed Jul 28, 2021

View reviewed changes

taminomara marked this pull request as ready for review July 28, 2021 15:20

taminomara force-pushed the ethcontract-rs-83 branch from 95f0add to 3614c14 Compare July 29, 2021 15:34

nlordell approved these changes Jul 30, 2021

View reviewed changes

vkgnosis approved these changes Jul 30, 2021

View reviewed changes

ethcontract-mock/src/details/mod.rs Outdated Show resolved Hide resolved

ethcontract-mock/src/details/mod.rs Outdated Show resolved Hide resolved

ethcontract-mock/src/details/parse.rs Show resolved Hide resolved

bh2smith approved these changes Jul 30, 2021

View reviewed changes

ethcontract-mock/src/details/default.rs Show resolved Hide resolved

taminomara force-pushed the ethcontract-rs-83 branch from 3614c14 to f668b2a Compare August 2, 2021 09:48

Tamika Nomara added 13 commits August 16, 2021 11:16

Implement Tokenize for Token

55f2544

Add Signature that will allow better type inference for mocks

9df5a84

Export method signatures for generated contracts

fc1f786

Add ethcontract-mock crate

21de911

Add trait that converts tuple of predicates into a predicate over tuples

9d73363

Add custom range implementation

e006e6a

Add mock interface

85070c5

Add module for mock implementation

7be665f

Add function to calculate default value for a solidity type

3424d84

Add function to verify transaction signature

c1b08c1

Add a helper to parse RPC call arguments

b82795d

Implement skeleton data structures for mock transport

05c7bee

Implement simple RPC calls

957b6b6

Tamika Nomara added 8 commits August 16, 2021 11:25

Implement method to add new expectations

2fcf77a

Implement transaction processing

ad631e4

Implement methods for configuring expectations

3d53aa2

Implement expectations verification and checkpoints

ccc47fa

Add documentation

15226f6

Add tests

e78fb0e

Run rustfmt

19a19bf

Update docs

e59df9d

taminomara force-pushed the ethcontract-rs-83 branch from 1c79fd6 to e59df9d Compare August 16, 2021 08:28

taminomara merged commit e59df9d into main Aug 16, 2021

taminomara deleted the ethcontract-rs-83 branch August 16, 2021 08:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Contract mock system #583

Contract mock system #583

taminomara commented Jul 13, 2021

taminomara commented Jul 13, 2021

taminomara commented Jul 26, 2021

fleupold left a comment

fleupold Jul 28, 2021

fleupold Jul 28, 2021

taminomara commented Jul 28, 2021

nlordell left a comment

nlordell Jul 30, 2021

nlordell Jul 30, 2021

nlordell Jul 30, 2021

taminomara Jul 30, 2021

nlordell Jul 30, 2021

nlordell Jul 30, 2021

taminomara Jul 30, 2021

nlordell Jul 30, 2021

nlordell Jul 30, 2021

nlordell Jul 30, 2021

nlordell Jul 30, 2021

vkgnosis left a comment

taminomara commented Jul 30, 2021

bh2smith left a comment

taminomara commented Aug 16, 2021

Contract mock system #583

Contract mock system #583

Conversation

taminomara commented Jul 13, 2021

taminomara commented Jul 13, 2021

taminomara commented Jul 26, 2021

fleupold left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taminomara commented Jul 28, 2021

nlordell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vkgnosis left a comment

Choose a reason for hiding this comment

taminomara commented Jul 30, 2021

bh2smith left a comment

Choose a reason for hiding this comment

taminomara commented Aug 16, 2021