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

Volt Code Review Takeaways #298

Closed
9 of 10 tasks
RusseII opened this issue May 12, 2022 · 7 comments
Closed
9 of 10 tasks

Volt Code Review Takeaways #298

RusseII opened this issue May 12, 2022 · 7 comments

Comments

@RusseII
Copy link
Contributor

RusseII commented May 12, 2022

@ElliotFriedman
Copy link

to expand on the assert test for GRACE_PERIOD, all state variables that get set in a constructor or during initialization need to be tested for correctness

@Namaskar-1F64F
Copy link
Contributor

@ElliotFriedman Did you look into the factory by chance? I forget the context there.

I think it was whether or not using a clone factory with the overhead of each call made sense.

Or maybe it was if it was possible at all. I think @RusseII might have run into trouble deploying at all with the size of our contracts + imports.

@RusseII
Copy link
Contributor Author

RusseII commented Jun 6, 2022

Or maybe it was if it was possible at all. I think @RusseII might have run into trouble deploying at all with the size of our contracts + imports.

I think the issue was if we did not use the clone factory then the contract size was too large for us to deploy. I think @ElliotFriedman had some issue like this in the past and was going to see if it was possible to deploy the bondFactory + bond code separately to reduce contract size with a regular factory?

@ElliotFriedman
Copy link

It really should be possible. Try running hardhat contract size on your contracts in their current state, then try doing another run of the contract size after you remove the creation of a new bond token in the factory. The factory bytecode should be smaller after you remove the creation of the new token.

@RusseII
Copy link
Contributor Author

RusseII commented Jun 13, 2022

@Namaskar-1F64F ready to close this?

@Namaskar-1F64F
Copy link
Contributor

Namaskar-1F64F commented Jun 13, 2022

It really should be possible. Try running hardhat contract size on your contracts in their current state, then try doing another run of the contract size after you remove the creation of a new bond token in the factory. The factory bytecode should be smaller after you remove the creation of the new token.

After removing the bond and the instantiation,

- import "./Bond.sol";
- tokenImplementation = address(new Bond());
+ tokenImplementation = address(0x0);

using hh size-contracts
CleanShot 2022-06-13 at 19 45 50@2x

@ElliotFriedman
Copy link

Nice!

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

3 participants