search

makerdao / dss-exec-lib

DSS Executive Spellcrafting Library Contracts
GNU Affero General Public License v3.0
36 stars 21 forks source link

Move COB from DssAction to DssExecLib #72

Closed brianmcmichael closed 3 years ago

brianmcmichael commented 3 years ago

Still wondering whether we should also import CollateralOpts into DssAction so that it's available to the spellcrafter, otherwise they will need to do a separate explicit import.

hexonaut commented 3 years ago

Looks good to merge after the visibility fix.

hexonaut commented 3 years ago

I think I prefer using the ABIEncoderV2 vs flattening into a bunch of arrays. Is there an issue with adding the V2 encoder pragma to enable struct serialization?

brianmcmichael commented 3 years ago

Yeah getting UnimplementedFeatureError out of solc when compiling with parameterized struct in ABIEncoderV2.

Looks something like:

./test.sh
 dapp-build: building with linked libraries
UnimplementedFeatureError: Encoding type "struct CollateralOpts memory" not yet implemented.

dapp-test: rpc block: latest
hevm: (^?!): empty Fold
CallStack (from HasCallStack):
  error, called at src/Control/Lens/Fold.hs:1310:28 in lens-4.18.1-9qmhqm9T86YJHOPOEopx4d:Control.Lens.Fold
  ^?!, called at src/EVM/Solidity.hs:342:20 in hevm-0.44.1-LcTWU2RNn7iMuZpzDnXLB:EVM.Solidity
Makefile:3: recipe for target 'test' failed
make: *** [test] Error 1

Haven't tested with solc:0.7.X but kind of wondering if it got straightened out by then.

gbalabasquer commented 3 years ago

Yeah getting UnimplementedFeatureError out of solc when compiling with parameterized struct in ABIEncoderV2.

Looks something like:

./test.sh
 dapp-build: building with linked libraries
UnimplementedFeatureError: Encoding type "struct CollateralOpts memory" not yet implemented.

dapp-test: rpc block: latest
hevm: (^?!): empty Fold
CallStack (from HasCallStack):
  error, called at src/Control/Lens/Fold.hs:1310:28 in lens-4.18.1-9qmhqm9T86YJHOPOEopx4d:Control.Lens.Fold
  ^?!, called at src/EVM/Solidity.hs:342:20 in hevm-0.44.1-LcTWU2RNn7iMuZpzDnXLB:EVM.Solidity
Makefile:3: recipe for target 'test' failed
make: *** [test] Error 1

Haven't tested with solc:0.7.X but kind of wondering if it got straightened out by then.

I was playing with it for a while and having the same error. It gets solved if you declare the experimental pragma also in the caller. That would mean the spell action itself should have the declaration as well. That's something to evaluate. Here a PR with the functionality working: https://github.com/makerdao/dss-exec-lib/pull/75

brianmcmichael commented 3 years ago

I was playing with it for a while and having the same error. It gets solved if you declare the experimental pragma also in the caller. That would mean the spell action itself should have the declaration as well. That's something to evaluate. Here a PR with the functionality working: #75

This may have been the problem. I hadn't included it in the test files, which after looking at your PR must have been the issue.

hexonaut commented 3 years ago

LGTM after https://github.com/makerdao/dss-exec-lib/pull/75 is merged in.