Tooling for tests v1 (StructAbiEncoder, computeHash functions) #11
Conversation
mitchelljustin
commented
Aug 3, 2018
mitchelljustin
commented
- New convenience class: StructAbiEncoder
- Extract helper functions: computeStateHash and computeActionHash.
- utils.ts: Rename some constants and remove some unused functions.
- Migrated commitReveal.spec.ts
mitchelljustin
changed the title
| } | ||
| definitions.push(definition); | ||
| } | ||
| const encoding = `tuple(${definitions.join(", ")})`; |
There was a problem hiding this comment.
It would be cool to extend this in the future to support encodings with nested tuples.
There was a problem hiding this comment.
Yeah, shouldn't be hard to do
| await multisig.deploy(masterAccount); | ||
| await masterAccount.sendTransaction({ | ||
| to: multisig.address, | ||
| value: Utils.UNIT_ETH.mul(2) | ||
| value: ethers.utils.parseEther("2") |
There was a problem hiding this comment.
Passing thought: maybe we can have a wrapper around this to abstractly indicate 2 eth being used instead of having a utility function needing to parse it?
There was a problem hiding this comment.
What do you mean? EtherAmount(2)?
There was a problem hiding this comment.
Yeah something like that - def not a blocker by any means tho
There was a problem hiding this comment.
I'm not sure that's an abstraction worth doing. Esp if all the other Ethereum tooling uses raw integers.
There was a problem hiding this comment.
Yeah it may add unneeded overhead to existing usable patterns. Again, just a passing thought - not worth reading too much into
There was a problem hiding this comment.
Maybe just a shorthand util
export const str2eth = ethers.utils.parseEther| @@ -91,7 +107,7 @@ export const setupTestEnv = (web3: any) => { | |||
| return { provider, unlockedAccount }; | |||
| }; | |||
|
|
|||
| export function signMessage(message, wallet): [number, string, string] { | |||
| function signMessage(message, wallet): [number, string, string] { | |||
There was a problem hiding this comment.
Why privatize these? Are they not being used anywhere else?
There was a problem hiding this comment.
Yup, they're not
ldct
changed the title
mitchelljustin
force-pushed
the
mitch/test-tooling
branch
from
5b38bef
to
f216394
Compare
| ); | ||
| app = { | ||
| const appContract = await deployContract(CommitRevealApp, masterAccount); | ||
| const app = { |
There was a problem hiding this comment.
We could write a type for this. addr is a 20 byte string, reduce could be a ethers.Interface.FunctionDefinition etc
There was a problem hiding this comment.
Already on it (next PR)
- ~Revert countingGame to develop version
mitchelljustin
force-pushed
the
mitch/test-tooling
branch
from
f216394
to
47ed006
Compare
