Gas optimization for Ubiquity contracts

[0xRizwan]

Hi,

I have done gas optimization on contracts in development branch.

Link to check the gas optimization changes- development...0xRizwan:ubiquity-dollar-Gas-optimization:development

Total gas saved from current implementation contracts- 13_38_283

cc- @pavlovcik and @molecula451

[0x4007]

How long does something like this take to do?

Also @rndquu @gitcoindev rfc

[rndquu]

How long does something like this take to do?

Also @rndquu @gitcoindev rfc

I would put ~4 hours

[molecula451]

i'm likely good with require() i would focus in gas optimizations in edges such as:

• Minting
• Deposit
• Withdraw

Overall user experience but at the same time preserving what we architecturally like in terms of code definition.

Perhaps we would like to upgrade to Erroring Support or not

[0xRizwan]

How long does something like this take to do?
Also @rndquu @gitcoindev rfc

I would put ~4 hours

Hey,

Thanks for the comment. This would definitely need ~6 hours. The tests also required to be changed. To be noted all tests are passed shared in above link.

[0xRizwan]

i'm likely good with require() i would focus in gas optimizations in edges such as:

• Minting
• Deposit
• Withdraw

Overall user experience but at the same time preserving what we architecturally like in terms of code definition.

Perhaps we would like to upgrade to Erroring Support or not

custom errors are preferred over require/assert. This definately saves gas with use of custom errors. In case of require, i see the string message length was more than 32 bytes at some instances which means additional 1 slot is required in such case. Instead of using more than 1 slot for just return string message, i would still prefer custom errors. To be noted, custom errors has not been used fully in all contracts, at some places require is kept too.

I believe the contracts is being deployed on Ethereum mainnet. For user experience, i would request to see the possibility of L2 deployment. This definately increases number of users using the protocol due to low fees and the users frequently using functions like Minting, Deposit, Withdraw would incur very very low gas as compared to Ethereum.

For example:
Taking my real life example. On average, I pay 4$ for Ethereum transaction and 0.25$ for Arbitrum transaction.

[molecula451]

custom errors are preferred over require/assert. This definately saves gas with use of custom errors. In case of require, i see the string message length was more than 32 bytes at some instances which means additional 1 slot is required in such case. Instead of using more than 1 slot for just return string message, i would still prefer custom errors. To be noted, custom errors has not been used fully in all contracts, at some places require is kept too.

I'm sure custom errors are better implementation, consider this as a refactor, but changing to custom errors could be considered a refactor, instead of some gas optimization as there are require() working very good tho, feel free to start working on the issue

[0xRizwan]

/start

[molecula451]

/help

[ubiquibot]

Available Commands

Command	Description	Example
/start	Assign yourself to the issue.	/start
/stop	Unassign yourself from the issue.	/stop
/help	List all available commands.	/help
/query	Returns the user's wallet, access, and multiplier information.	/query @user
/ask	Ask a context aware question.	/ask is x or y the best approach?
/multiplier	Set the task payout multiplier for a specific contributor, and provide a reason for why.	/multiplier @user 0.5 "multiplier reason"
/labels	Set access control, for admins only.	/labels @user priority time price
/authorize	Approve a label change, for admins only.	/authorize
/wallet	Register your wallet address for payments.	/wallet ubq.eth

[0x4007]

@0xRizwan Make sure to set your wallet address first. I think that's the unhandled error.

[0xRizwan]

/wallet 0x9Ea3efa3F1145A46c4eEc34B5a995De570b8050b

[ubiquibot]

+ Successfully registered wallet address

[0xRizwan]

/start

[ubiquibot]

Deadline	Wed, Feb 14, 5:01 AM UTC
Registered Wallet	0x9Ea3efa3F1145A46c4eEc34B5a995De570b8050b

Tips:

• Use /wallet 0x0000...0000 if you want to update your registered payment wallet address.
• Be sure to open a draft pull request as soon as possible to communicate updates on your progress.
• Be sure to provide timely updates to us when requested, or you will be automatically unassigned from the task.

[molecula451]

I believe the contracts is being deployed on Ethereum mainnet. For user experience, i would request to see the possibility of L2 deployment. This definately increases number of users using the protocol due to low fees and the users frequently using functions like Minting, Deposit, Withdraw would incur very very low gas as compared to Ethereum.

Hey We are going Mainnet, so consider that do not take in count L2s

[molecula451]

custom erros can also be inside require() that's why the change could be considered a refactor

[0xRizwan]

Please check PR

Overall gas change from current implementation: 6.6 million which is ~2.3 % change.

Feel free to connect for any clarifications.

[ubiquibot]

# These linked pull requests are closed:  <a href="https://github.com/ubiquity/ubiquity-dollar/pull/896">#896</a> 

[ubiquibot]

! action has an uncaught error

[ubiquibot]

! action has an uncaught error

[0x4007]

I'll try salvaging the permits from the logs here.

[ubiquibot]

+ Evaluating results. Please wait...

[0x4007]

[ 18.3 WXDAI ]

@pavlovcik

Contributions Overview

View	Contribution	Count	Reward
Issue	Comment	3	3.7
Review	Comment	3	14.6

Conversation Incentives

Comment	Formatting	Relevance	Reward
How long does something like this take to do? Also @rndquu @git...	1.3	0.49	1.3
@0xRizwan Make sure to set your wallet address first. I think th...	1.6	0.44	1.6
I'll try debugging the permit flow here. ...	0.8	0.45	0.8
> require used with return string message adds more readabilit...	4.4 a: count: 1 score: "1" words: 1 code: count: 1 score: "1" words: 1	0.75	4.4
> i think we could perhaps compensate the user for these comment...	4.4	0.57	4.4
Yes unfortunately the permit flow is a coin toss right now. I th...	5.8 li: count: 1 score: "1" words: 8	0.56	5.8

[ 29.5 WXDAI ]

@molecula451

Contributions Overview

View	Contribution	Count	Reward
Issue	Comment	4	18.3
Review	Comment	3	11.2

Conversation Incentives

Comment	Formatting	Relevance	Reward
i'm likely good with `require()` i would focus in gas optimizati...	8.8 li: count: 3 score: "3" words: 3 code: count: 1 score: "1" words: 1	0.7	8.8
> custom errors are preferred over require/assert. This definate...	5.4 code: count: 1 score: "1" words: 1	0.73	5.4
> I believe the contracts is being deployed on Ethereum mainnet....	1.4	0.45	1.4
custom erros can also be inside `require()` that's why the chang...	2.7 code: count: 1 score: "1" words: 1	0.51	2.7
Please fix the build actions as some are not passing...	1	0.31	1
Yeah i think the PR won't make it either as the such refactors s...	7.7	0.68	7.7
it looks like it took action on the PR closing but not on the is...	2.5	0.59	2.5

[ 21.2 WXDAI ]

@gitcoindev

Contributions Overview

View	Contribution	Count	Reward
Review	Comment	1	21.2

Conversation Incentives

Comment	Formatting	Relevance	Reward
Hi everyone, I am back. I analysed the changes. Now I see where ...	21.2 li: count: 3 score: "3" words: 43	0.61	21.2

[ 89.8 WXDAI ]

@0xRizwan

Contributions Overview

View	Contribution	Count	Reward
Issue	Specification	1	7.2
Issue	Comment	3	44.6
Review	Comment	2	38

Conversation Incentives

Comment	Formatting	Rele

[0x4007]

Looks like the qualitative analysis is no longer applying its multiplier. I see on gitcoindev's comment that it clearly is not doing anything: raw score of 21.2 with a relevance of 0.61 should yield 12.932

Looks like I already filed the issue here

• Qualitative Score Not Applying Correctly ubiquibot/comment-incentives#8

[0xRizwan]

@pavlovcik @molecula451 and team,

Thank you.