WIP: support multiple balance formats in targetContractsBalances

[0xZRA]

This is to address issue #306
Relies on running go generate command to automatically generate code generate JSON marshaling code from template for FuzzingConfig

@anishnaik

I'm running into an issue where tests are failing locally on my machine, even on the master branch. I wanted to flag this early - I'm using Sequoia version 15.1.1, so it's unclear if my changes are implemented correctly.

go test -v ./fuzzing

FAIL
FAIL    github.com/crytic/medusa/fuzzing        240.868s
FAIL

At the same time, when running following tests individually thru VSCode, both are passing:

Running tool: /opt/homebrew/bin/go test -timeout 30s -run ^TestDeploymentsWithPayableConstructors$ github.com/crytic/medusa/fuzzing

ok  	github.com/crytic/medusa/fuzzing	6.693s

Running tool: /opt/homebrew/bin/go test -timeout 30s -run ^TestDeploymentsWithPredeploy$ github.com/crytic/medusa/fuzzing

ok  	github.com/crytic/medusa/fuzzing	1.280s

[CLAassistant]

All committers have signed the CLA.

[anishnaik]

So the beauty of using go generate is that we can still represent TargetContractBalances as []*big.Int but marshal/unmarshal it as strings. So, you actually do not need this change. You can keep it as the original.

[anishnaik]

You won't need this logic once the change in config.go is made

[anishnaik]

So technically the only change you have to make for this PR is this (and the update to the documentation). Everything else should click into place once go generate is run in the config/ directory.

[0xZRA]

@anishnaik I'm getting the following error when running go generate from config dir:

rebase@MacBookPro config % go generate                                                              
-: invalid field override: slice element type string is not convertible to *math/big.Int
exit status 1
config.go:22: running "go": exit status 1

[anishnaik]

@0xZRA provided some feedback. Lmk if you have any questions. I'll run the CI when you make those changes and I can investigate if/why the tests are failing.

[anishnaik]

Hey @0xZRA, will provide some more feedback soon. Sorry for the delay.

[anishnaik]

Hey @0xZRA okay so I think we need to change our approach. Tbh I didn't expect go generate to complain about this (sorry about that). So, going back to the original problem: we need to (de)serialize the big.Ints. Another way to approach this is to create a custom type (type ContractBalance *big.Int). Thus, the TargetContractBalances will become of type []ContractBalance. Now, we can write our own custom MarshalJSON and UnmarshalJSON that will marshal the ContractBalance to a string and unmarshal from a string to a *big.Int. Here is a stack overflow post that should get you in the right direction: https://stackoverflow.com/questions/53991835/how-to-marshal-and-unmarshal-big-int-in-json

Doing it this way will remove any requirement for go generate so you can remove the directive and the associated file as well.

I would like to take it one more step than what is mentioned in the stack overflow post. I would like to be able to unmarshal any three of these string possibilities:

1. Base-10 string: "1234" or "45678"
2. Hex string: "0xabcdef"
3. Scientific notation for base-10 strings (optional if it is easy): "1.2e18"

Let me know if you have any questions.

Also, about the failing tests: We have some challenging unit tests that sometimes takes medusa a while to solve. That's why sometimes the tests may fail to run. Usually, re-running it does the trick but I do need to investigate why it is happening more frequently nowadays.

[0xZRA]

Hey @0xZRA okay so I think we need to change our approach. Tbh I didn't expect go generate to complain about this (sorry about that). So, going back to the original problem: we need to (de)serialize the big.Ints. Another way to approach this is to create a custom type (type ContractBalance *big.Int). Thus, the TargetContractBalances will become of type []ContractBalance. Now, we can write our own custom MarshalJSON and UnmarshalJSON that will marshal the ContractBalance to a string and unmarshal from a string to a *big.Int. Here is a stack overflow post that should get you in the right direction: https://stackoverflow.com/questions/53991835/how-to-marshal-and-unmarshal-big-int-in-json

Doing it this way will remove any requirement for go generate so you can remove the directive and the associated file as well.

I would like to take it one more step than what is mentioned in the stack overflow post. I would like to be able to unmarshal any three of these string possibilities:

1. Base-10 string: "1234" or "45678"
2. Hex string: "0xabcdef"
3. Scientific notation for base-10 strings (optional if it is easy): "1.2e18"

Let me know if you have any questions.

Also, about the failing tests: We have some challenging unit tests that sometimes takes medusa a while to solve. That's why sometimes the tests may fail to run. Usually, re-running it does the trick but I do need to investigate why it is happening more frequently nowadays.

@anishnaik I appreciate your response and I think it makes sense so far. Just wanted to clarify one point in your approach regarding the unmarshalling of the 3 string possibilities. Does the proposed approach still involve maintaining the current hexadecimal notation for setting initial balances, while adding the option to use base-10 strings, including scientific notation?

So if we take it straight to the docs, the new version will look like:

targetContractsBalances

• Type: [Base-16 Strings] (e.g. [0x123, 0x456, 0x789])
• Type: [Base-10 Strings] (e.g. ["123", "1000000", "115792089237316195423570985008687907853269984665640564039457584007913129639935"])
• Type: [Base-10 Strings Scientific Notation] (e.g. ["1.23e2", "1.0e6", "1.15792089237316195423570985008687907853269984665640564039457584007913129639935e77"])

[anishnaik]

Technically, you can also mix and match: ["1e12", "0x1234", "5678"]. Since you are evaluating each string individually, and not the whole array, the logic shouldn't change depending on what the array looks like.

Like the UnmarshalJSON function you write will take a []byte and you need to convert it into a ContractBalance (aka *big.Int). This []byte could be any of the three string possibilities (or invalid).

Also, for MarshalJSON, always just serialize it as a base-10 string.

This PR became more complicated than I expected :)

[0xZRA]

Technically, you can also mix and match: ["1e12", "0x1234", "5678"]. Since you are evaluating each string individually, and not the whole array, the logic shouldn't change depending on what the array looks like.

Like the UnmarshalJSON function you write will take a []byte and you need to convert it into a ContractBalance (aka *big.Int). This []byte could be any of the three string possibilities (or invalid).

Also, for MarshalJSON, always just serialize it as a base-10 string.

This PR became more complicated than I expected :)

I hadn't thought of mix and match, you're right. I'll take a shot at it.

[0xZRA]

This is still WIP, as I haven't tested the actual runs with different balance formats. @anishnaik let me know if I captured the requirements correctly