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

chore(protocol): update Alpha-3 protocol configurations #13806

Merged
merged 7 commits into from
May 26, 2023

Conversation

davidtaikocha
Copy link
Member

@davidtaikocha davidtaikocha commented May 24, 2023

Questions to discuss:

  • What average block time will we use / assume for A3? (3 seconds?) => 3s seems to be reasonable
  • Shall we change minEthDepositAmount to 0.1 ETHs? (some concerns here) => should be fine
  • Do we need to increase the cooldown period for A3 network? (proofCooldownPeriod + systemProofCooldownPeriod) => if Sepolia generally work well and we're confident we can get these overwrite transactions in fast enough then seems ok.
  • Do we need to change realProofSkipSize? (i think it depends on the avg block time that we predict) => start with realProofSkipSize: 10 for the initial wave, we can reconsider afterwards
  • What treasury address will we use? => 0xdf09A0afD09a63fb04ab3573922437e1e637dE8b
  • What INITIAL_PROOF_TIME_TARGET will we use? (in current internal devnet, avg proof submission time is ~80s, so shall we use 120s? if we use 120s, then initProofTimeIssued will be 49883) => 2 * 80 (avg proof submission time) = 160
  • What param1559.gasIssuedPerSecond will we use? (i think it depends on the avg block time that we predict) => param1559.gasIssuedPerSecond := 2,000,000 gas/s == 4 L2 blocks of 6M gas/L1 block
  • Shall we change param1559.yscale / param1559.xscale / param1559.gasExcess? => lets calculate them in TaikoL2's init() function
  • Shall we make sure state.blockFee always >=1? (to prevent proposing becomes "totally free"?) => If the proposing EIP-1559 parameters are set well, seems this will be unnecessary (could be a big IF)

github-actions[bot]

This comment was marked as off-topic.

packages/protocol/contracts/L1/TaikoConfig.sol Outdated Show resolved Hide resolved
packages/protocol/script/DeployOnL1.s.sol Outdated Show resolved Hide resolved
packages/protocol/script/DeployOnL1.s.sol Outdated Show resolved Hide resolved
packages/protocol/test/genesis/test_config.json Outdated Show resolved Hide resolved
packages/protocol/test/genesis/test_config.json Outdated Show resolved Hide resolved
Copy link
Contributor

@Brechtpd Brechtpd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because each proof has the exact same cost currently, I think we should make it so the prover fee cannot change quickly (tweak adjustmentQuotient).

What average block time will we use / assume for A3? (3 seconds?)

Very important this matches the proposing EIP-1559 target!

Do we need to increase the cooldown period for A3 network? (

I don't know how stable Sepolia is, but if things generally work well and we're confident we can get these overwrite transactions in fast enough then seems ok to me.

What param1559.gasIssuedPerSecond will we use? (i think it depends on the avg block time that we predict)

EDIT: I confused this value with gasExcess.

Shall we change param1559.yscale / param1559.xscale / param1559.gasExcess?

See comment on this above.

Shall we make sure state.blockFee always >=1? (to prevent proposing becomes "totally free"?)

If the proposing EIP-1559 parameters are set well, I don't think this will be necessary (could be a big IF)

packages/protocol/contracts/L1/TaikoConfig.sol Outdated Show resolved Hide resolved
packages/protocol/script/DeployOnL1.s.sol Outdated Show resolved Hide resolved
packages/protocol/test/genesis/test_config.json Outdated Show resolved Hide resolved
@Brechtpd
Copy link
Contributor

Brechtpd commented May 24, 2023

  • Maybe we should do minEthDepositsPerBlock: 1? Feels like that could be a big UX improvement.
  • I think smart to start with realProofSkipSize: 10 for the initial wave, we can reconsider afterwards.
  • Regardless of the value of minEthDepositAmount, I think there's definitely spam risk on this queue. Anybody with a lot of ETH can make the queue extremely long and we can only process a couple/block, so users may have to wait a long time to get their ETH deposit if abused.
  • INITIAL_PROOF_TIME_TARGET = 80s * 2
  • prover fee adjustmentQuotient: feat(protocol): Scale up damping factor and flatten curve #13809
  • param1559.gasIssuedPerSecond := 2,000,000 gas/s == 4 L2 blocks of 6M gas/L1 block. Seems reasonable to me and is the proposed 3s block times. Other values:
struct EIP1559Params {
        basefee = ? (could just be 0 to start);
        gasIssuedPerSecond = 2000000;
        gasExcessMax = ?;
        gasTarget = 2000000 * 12;
        ratio2x1x = 11250;
    }

Unfortunately not so easy to find all the correct values here because some manual tweaking necessary in the helper script which I haven't tried to do. Something easier and fully automatic like https://github.com/taikoxyz/taiko-mono/blob/main/packages/protocol/script/DetermineNewProofTimeIssued.s.sol + some input parameter simplification #13556 like with the proving fee would certainly help to make this easier to configure!

@davidtaikocha
Copy link
Member Author

davidtaikocha commented May 25, 2023

Updated the configs based on PR feedbacks: 1f6eed5 , plz review @Brechtpd @dantaik

Brechtpd
Brechtpd previously approved these changes May 25, 2023
Copy link
Contributor

@Brechtpd Brechtpd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we seen the proposal basefee work within reasonable ranges on our internal testnet?

@davidtaikocha davidtaikocha marked this pull request as ready for review May 25, 2023 10:37
@davidtaikocha
Copy link
Member Author

Have we seen the proposal basefee work within reasonable ranges on our internal testnet?

In current internal devnet, all provers will at least wait proofTargetTime before submission, but right now the baseFee is still 0, and i think @adaki2004 is working on tuning the parameters.

@Brechtpd
Copy link
Contributor

Ah yeah I heard about that!

It looks like how I name things is a bit confusing, what about the L2 basefee that the proposer pays on L2? That one also depends on proposing blocks at fixed intervals (same as with the proofs) to keep the basefee steady. So was wondering if the behavior there was as expected and we can for sure push that L2 basefee pretty high in a small amount of time (we need that high L2 basefee to prevent spam).

@dantaik
Copy link
Contributor

dantaik commented May 25, 2023

@davidtaikocha @Brechtpd @adaki2004 Do you think we should get onto a call to quickly resolve this issue?

@davidtaikocha
Copy link
Member Author

davidtaikocha commented May 25, 2023

@davidtaikocha @Brechtpd @adaki2004 Do you think we should get onto a call to quickly resolve this issue?

image

after upgrade the proxy implementation and use Dani's new params:

ProofTimeTarget: 150
ProofTimeIssued: 162263074
BlockFee: 1e8 ( 1 TKO was set so far so i guess we stick to it.. if you disagree i need to recalculate the proofTimeIssued)
AdjustmentQuotient: 32000

seems the fee backs to stable again at least for last hour. and before that, our client sometimes will submit the proofs asap, and zkpool's client also always submits proof asap, i think this is also part of the reason.

@davidtaikocha
Copy link
Member Author

davidtaikocha commented May 25, 2023

@davidtaikocha @Brechtpd @adaki2004 Do you think we should get onto a call to quickly resolve this issue?

And im ok for a call to do the final overall checking of this PR.

@dantaik dantaik enabled auto-merge May 26, 2023 00:03
@dantaik dantaik requested review from adaki2004 and removed request for d1onys1us, adaki2004 and cyberhorsey May 26, 2023 00:05
@dantaik dantaik added this pull request to the merge queue May 26, 2023
Merged via the queue into alpha-3 with commit 3298466 May 26, 2023
@dantaik dantaik deleted the update-alpha-3-configs branch May 26, 2023 02:53
d1onys1us added a commit that referenced this pull request Jun 5, 2023
@Mehrabani61
Copy link

#13483

@Mehrabani61
Copy link

Duplicate of #

@Mehrabani61
Copy link

update-alpha-3-configs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update configs in TaikoConfig.sol \ EIP1559Params for Alpha 3 testnet
6 participants