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.

Sign up for GitHub

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

Jump to bottom

Incorrect estimation and high gas costs on first call to deposit::topupDeposit() #1991

Open
DrZoltanFazekas opened this issue Dec 9, 2024 · 3 comments · May be fixed by #2102
Open

Incorrect estimation and high gas costs on first call to deposit::topupDeposit() #1991

DrZoltanFazekas opened this issue Dec 9, 2024 · 3 comments · May be fixed by #2102
Assignees
@86667
Labels
Agate Required for mainnet launch

Comments

@DrZoltanFazekas
Copy link
Contributor

When the deposit contract's topupDeposit() function is called for the very first time (in the example below from a proto-testnet delegation contract's stake() function) it uses a large amount of gas:

[Success]Hash: 0x35168b94728dd1182ef88e52ec7447bff380c2a9c79fc0ebf2afe0ab44994314
Block: 7777350
Paid: 5.2112190893088 ETH (1094356 gas * 4761.9048 gwei)

When it's called for the second time by another staker's control address (i.e. in the example below by another delegation contract) it uses one order of magnitude less gas:

[Success]Hash: 0x5c81ccf531a88c020fc88598fb5d36c644868bf100d3955445ffb0b41226511b
Block: 7777407
Paid: 0.8082523874184 ETH (169733 gas * 4761.9048 gwei)

Finally, each time it's called by the same staker's control address again, it uses a constant amount of gas:

[Success]Hash: 0xe6b01ba749c3cbdcdecb9f83d9ea23c9d91b8734c6b331a4b063e6d4e7d7773e
Block: 7777451
Paid: 0.6453952432584 ETH (135533 gas * 4761.9048 gwei)

Most likely the first initialization of storage slots makes the very first call so expensive. Investigate how to make the distribution of gas costs across the calls more fair.

Another issue that was observed is that if the first call was in a transaction submitted from the Rabby wallet, the transaction ran out of gas i.e. the estimation delivered a gas limit that was too low to execute the transaction. If the transaction was submitted using a Forge script like above, the estimated gas limit was sufficient though.
@DrZoltanFazekas DrZoltanFazekas changed the title High gas costs on first call to deposit::topupDeposit() Incorrect estimation and high gas costs on first call to deposit::topupDeposit() Dec 9, 2024
@DrZoltanFazekas DrZoltanFazekas added the Agate Required for mainnet launch label Dec 9, 2024
@DrZoltanFazekas
Copy link
Contributor Author

@86667 If you look at the traces of the stake() transactions of this delegation contract deployed on the proto-testnet: https://explorer.zq2-prototestnet.zilliqa.com/address/0x7A28eda6899d816e574f7dFB62Cc8A84A4fF92a6

you will see that the only thing that makes a huge difference is the amount of gas used in the depositTopup() function of the deposit contract:

        {
            "type": "call",
            "action": {
                "from": "0x00000000005a494c4445504f53495450524f5859",
                "callType": "delegatecall",
                "gas": "0x13ee0c",
                "input": "0x90948c25",
                "to": "0x47c5e40890bce4a473a49d7501808b9633f29782",
                "value": "0x5b12aefafa8040000"
            },
            "result": {
                "gasUsed": "0x1038d0",
                "output": "0x"
            },
            "subtraces": 0,
            "traceAddress": [
                0,
                6,
                0
            ]
        }

        {
            "type": "call",
            "action": {
                "from": "0x00000000005a494c4445504f53495450524f5859",
                "callType": "delegatecall",
                "gas": "0x128e3",
                "input": "0x90948c25",
                "to": "0x47c5e40890bce4a473a49d7501808b9633f29782",
                "value": "0x5b12aefafa8040000"
            },
            "result": {
                "gasUsed": "0x6781",
                "output": "0x"
            },
            "subtraces": 0,
            "traceAddress": [
                0,
                6,
                0
            ]
        }

        {
            "type": "call",
            "action": {
                "from": "0x00000000005a494c4445504f53495450524f5859",
                "callType": "delegatecall",
                "gas": "0x1b09e6",
                "input": "0x90948c25",
                "to": "0x47c5e40890bce4a473a49d7501808b9633f29782",
                "value": "0x56bc75e2d63100000"
            },
            "result": {
                "gasUsed": "0xa84b9",
                "output": "0x"
            },
            "subtraces": 0,
            "traceAddress": [
                0,
                6,
                0
            ]
        }

        {
            "type": "call",
            "action": {
                "from": "0x00000000005a494c4445504f53495450524f5859",
                "callType": "delegatecall",
                "gas": "0x67649",
                "input": "0x90948c25",
                "to": "0x47c5e40890bce4a473a49d7501808b9633f29782",
                "value": "0x6046f37e5945c0000"
            },
            "result": {
                "gasUsed": "0x65972",
                "output": "0x"
            },
            "subtraces": 0,
            "traceAddress": [
                0,
                6,
                0
            ]
        }

        {
            "type": "call",
            "action": {
                "from": "0x00000000005a494c4445504f53495450524f5859",
                "callType": "delegatecall",
                "gas": "0xac1e2",
                "input": "0x90948c25",
                "to": "0x47c5e40890bce4a473a49d7501808b9633f29782",
                "value": "0x21e19e0c9bab2400000"
            },
            "result": {
                "gasUsed": "0xa9a99",
                "output": "0x"
            },
            "subtraces": 0,
            "traceAddress": [
                0,
                6,
                0
            ]
        }

@86667
Copy link
Contributor

86667 commented Jan 7, 2025 •
edited
Loading

@DrZoltanFazekas thanks. I understand where the cost is coming from and have reduced it here in this PR.

Still would like to reduce it further if I can. Review request incoming

@DrZoltanFazekas
Copy link
Contributor Author

Suggest you create a branch for this and other upcoming deposit contract changes (e.g. #2057 and fixes for audit findings) in the zq2 repo. We'll need another upgrade to deploy them on the proto-networks...

@86667 86667 linked a pull request Jan 7, 2025 that will close this issue
feat: deposit_v4 with cheaper depositTopUp #2102
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@86667 86667

Labels
Agate Required for mainnet launch
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: deposit_v4 with cheaper depositTopUp Zilliqa/zq2
2 participants
@86667 @DrZoltanFazekas