Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Too much automation within the smart contract; move it up a level #49

Open
GriffGreen opened this issue May 1, 2017 · 0 comments
Open

Comments

@GriffGreen
Copy link
Member

The automation is the scariest feature of this contract. It might be a smarter design choice to put automation into a deployment script outside of the smart contract whenever possible.

For instance in setVaultLimits()

It is probably unnecessary to include:

 if (address(parentVaultController) != 0) {
            parentVaultController.topUpVault();
            sendBackOverflow();
        }

This could easily be done as a script run when a user uses the UI to set the limits as a second function call, it might even save gas by moving some calculations outside of the smart contract.

But the main issue is security; topUpVault() is called way too many times all over this contract. The example above is just the most obvious one that seemed out of place. It is a relatively dangerous function to call so liberally.

I do appreciate that this could impact user experience (multiple function calls could mean, having to click metamask's accept button multiple times) but that is why we can debate the merits of this idea below :-D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant