ajna-finance / ajna-core

The Ajna protocol is a non-custodial, peer-to-peer, permissionless lending, borrowing and trading system that requires no governance or external price feeds to function.
https://www.ajna.finance/
Other
31 stars 11 forks source link

Optional: Reduce contract size and deployment cost. #935

Closed brianmcmichael closed 1 year ago

brianmcmichael commented 1 year ago

Description of change

High level

Description of bug or vulnerability and solution

Contract size

Pre Change

src/PositionManager.sol:PositionManager contract
Deployment Cost Deployment Size
3852018 19835

Post Change

src/PositionManager.sol:PositionManager contract
Deployment Cost Deployment Size
3796343 19557

Gas usage

Pre Change

src/PositionManager.sol:PositionManager contract
Function Name min avg median max # calls
DOMAIN_SEPARATOR 732 732 732 732 9
PERMIT_TYPEHASH 426 426 426 426 8
approve(address,uint256) 25302 25302 25302 25302 2
approve(address,uint256)(bool) 2722 20386 25302 25302 5
burn 1838 7886 9614 11214 29
getLP 6990 9646 8160 36718 110
getPositionIndexes 1374 2469 3374 3374 115
getPositionInfo 1053 1613 1053 5053 164
isIndexInPosition 1066 1629 1066 3066 71
isPositionBucketBankrupt 6870 7886 8090 9658 18
memorializePositions 6459 75345 73461 364200 3089
mint 10462 81694 79384 100184 105
moveLiquidity 5365 200914 206538 671265 30
nonces 841 1817 1746 2841 8
ownerOf 836 841 836 882 179
permit 1290 22654 17585 44417 9
poolKey 807 807 807 807 117
redeemPositions 2855 50356 60311 141906 64
safeTransferFrom 39704 41224 41704 41704 5
tokenURI 3157 468592 468592 934028 2
transferFrom(address,address,uint256) 40056 40494 40494 40932 2
transferFrom(address,address,uint256)(bool) 22536 33974 39332 40056 3

Post Change

src/PositionManager.sol:PositionManager contract
Function Name min avg median max # calls
DOMAIN_SEPARATOR 732 732 732 732 9
PERMIT_TYPEHASH 426 426 426 426 8
approve(address,uint256) 2722 20386 25302 25302 5
approve(address,uint256)(bool) 25302 25302 25302 25302 2
burn 1859 7913 9641 11241 29
getLP 6990 9637 8160 36718 103
getPositionIndexes 1374 2469 3374 3374 115
getPositionInfo 1053 1460 1053 5053 157
isIndexInPosition 1066 1629 1066 3066 71
isPositionBucketBankrupt 6870 7886 8090 9658 18
memorializePositions 6480 75379 73495 364228 3089
mint 10462 81694 79384 100184 105
moveLiquidity 5386 200942 206565 671299 30
nonces 841 1817 1746 2841 8
ownerOf 836 841 836 882 179
permit 1290 23250 17585 44417 9
poolKey 807 807 807 807 117
redeemPositions 2876 50383 60338 141933 64
safeTransferFrom 39704 41224 41704 41704 5
tokenURI 3157 468592 468592 934028 2
transferFrom(address,address,uint256) 40056 40494 40494 40932 2
transferFrom(address,address,uint256)(bool) 22536 33974 39332 40056 3
prateek105 commented 1 year ago

Is this worth the added maintenance complexity and gas cost of an extra method?

I misinterpreted the gas cost, I thought that also decreased along with the contract size. Since there is no issue with contract size as of now we can leave this PR for future in case a need arises.

EdNoepel commented 1 year ago

Per today's discussion, closing but leaving branch in-place in case we need to reduce contract size.