makerdao / dss-flash

MakerDAO Flash Mint Module
GNU Affero General Public License v3.0
50 stars 36 forks source link

opt: Optimize gas usage for locked modifier #14

Closed hexonaut closed 3 years ago

hexonaut commented 3 years ago

PR for https://github.com/hexonaut/dss-flash/issues/9

Making this switch saves 19,200 gas in my personal test.

gbalabasquer commented 3 years ago

Hmm if you are using dapptools for checking this gas usage you need to be careful, as you won't catch the set storage to 0 refund. I think the final gas cost should be the same for both type operations. Best way to test this specific case is deploying both contracts in kovan and actually checking a real tx.

I'd advise doing it before merging as I think it looks better the 0/1 switch and it also avoids the set in the constructor.

hexonaut commented 3 years ago

Just ran a test: https://kovan.etherscan.io/address/0xb89a0B376c12f9A27f481614cB01Acd6dBc79a2d#code

Existing lock1: https://kovan.etherscan.io/tx/0xff41332b0600d4ec0cbfe8928d1e77fbd9bcc707494e72cdcbd8860bd5851c86 Proposed new lock2: https://kovan.etherscan.io/tx/0xf0b40fcf158daa25f234b2a7a3aafa3039a09ea6f23557727f059a841d2d4765

Difference in gas is 22 which is probably just due to function call ordering or something. Looks like you are right @gbalabasquer. I'll close this PR.