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

CDK PP Docs Review Env: DO NOT MERGE!!! #2459

Merged
merged 25 commits into from
Nov 21, 2024

Conversation

EmpieichO
Copy link
Contributor

@EmpieichO EmpieichO commented Oct 23, 2024

Hosted url: docs-dev.polygon.technology/2459
CDK PP docs review env: DO NOT MERGE!!!

PR number Doc to review URL in Review Env Sidebar label
2460 docs/cdk/releases/pp-intro-components.md https://docs-dev.polygon.technology/2459/cdk/releases/pp-intro.components/ Pessimistic proofs (PP)
2461 docs/cdk/getting-started/cdk-erigon/resources.md https://docs-dev.polygon.technology/2459/cdk/getting-started/cdk-erigon/resources/ CDK Erigon/ Resources
2462 docs/cdk/architecture/cdk-pp-highlevel-arch.md https://docs-dev.polygon.technology/2459/cdk/architecture/cdk-pp-highlevel-arch/ CDK PP high level view
2463 docs/cdk/getting-started/cli-tool.md https://docs-dev.polygon.technology/2459/cdk/getting-started/cli-tool/ CLI tool

@EmpieichO EmpieichO changed the title CDK PP docs review env: DO NOT MERGE!!! CDK PP Docs Review Env: DO NOT MERGE!!! Oct 23, 2024
Adding new doc: `pp-intro.components.md`
Adding "Pessimistic proofs" to nav
@EmpieichO
Copy link
Contributor Author

Hi @sshrihar

I have created a Dev Env here https://docs-dev.polygon.technology/2459

Can you tell me why clicking on the /cdk/Pessismistic proofs/ throws a 404 ?

Fixing filename ref in nav pp-intro-components.md --> pp-intro.components.md
Remove DAC component
fixing indentation
Update resources.md in the cdk-erigon docs
Add CDK PP high level architecture doc
Update CLI Tool doc
@sshrihar
Copy link
Collaborator

Hi @sshrihar

I have created a Dev Env here https://docs-dev.polygon.technology/2459

Can you tell me why clicking on the /cdk/Pessismistic proofs/ throws a 404 ?

I tried accessing the url and it redirected me to https://docs-dev.polygon.technology/2459/cdk/releases/pp-intro.components/

@vcastellm vcastellm marked this pull request as draft October 30, 2024 17:39
docs/cdk/architecture/cdk-pp-highlevel-arch.md Outdated Show resolved Hide resolved
docs/cdk/architecture/cdk-pp-highlevel-arch.md Outdated Show resolved Hide resolved
docs/cdk/architecture/cdk-pp-highlevel-arch.md Outdated Show resolved Hide resolved
| Node = RPC and sequencer | [cdk-erigon](https://github.com/0xPolygonHermez/cdk-erigon) | Customizable, commonly:<br/>\- Sequencer = 1 node<br/>\- RPC = multiple nodes |
| Contracts | <a href=https://github.com/0xPolygonHermez/zkevm-contracts>zkevm-contracts</a> | |
| CLI | [cdk](https://github.com/0xPolygon/cdk) | Included in [CDK](https://github.com/0xPolygon/cdk) repo |
| AggSender | <a href=https://github.com/0xPolygon/cdk>cdk</a> | Included in [CDK](https://github.com/0xPolygon/cdk) repo |
Copy link
Contributor

Choose a reason for hiding this comment

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

The AggSender is just a sub-component of the cdk client. I think that we should be talking about cdk client or cdk node more in the doc... Although it's ok to specify that the AggSender is the specific sub-component that sends the certs to the AL. But note that the AggSender works in conjunction with other sub-components of the cdk such as the bridge syncer and others...

- AggSender is the CDK PP component that accumulates all necessary info in order to generate certificates, which are sent to the SP1 prover via the JSON-RPC API for the generation of pessimistic proofs.
- Transaction pool manager: For storing transactions submitted by users.

### AggLayer-side components
Copy link
Contributor

Choose a reason for hiding this comment

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

It feels a bit odd to have a sub section for AL, and not for CDK

- `PolygonZkEVMDeployer`
- `PolygonZkEVMTimelock`
- CLI tool: A single command line interface tool for abstracting away the complexity of deploying or configuring CDK components.
- AggSender is the CDK PP component that accumulates all necessary info in order to generate certificates, which are sent to the SP1 prover via the JSON-RPC API for the generation of pessimistic proofs.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like the idea of CDK PP. No, there is the cdk. And the cdk happens to be used to give PP functionality, but there is no CDK PP... Again, once we've FEP+PP the same cdk will be used, and in this case we will have the aggsender as well as other components


We henceforth refer to CDK sovereign chains as CDK PP chains.

## CDK PP stack
Copy link
Contributor

Choose a reason for hiding this comment

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

Overall, there is a weird level of detail... We list the CLI, but not the syncers? Most importantly, the aggSender looks like a standalone piece of software, when it's clearly just a functionality of the cdk client. I've been talking with some non Polygon folks and they're super confused with what's the CDK / AggLayer. We need to be very careful on how we articulate this things...


Unlike in the CDK FEP chain, the prover in a CDK PP chain is not directly connected to the chain. It in fact resides in the AggLayer.

- JSON-RPC API: A component provided by the AggLayer to interface between the CDK PP chain and the SP1 prover.
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldnt it be better to talk about AggLayer node similar to the aggsender, the RPC is just a part of the AL node... but for sure avoid JSON-RPC it's extremely confusing, people will think about the API of the Ethereum execution clients when they see this, and it has nothing to do with it

Choose a reason for hiding this comment

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

How do I discord you guys

Copy link

sonarcloud bot commented Nov 21, 2024

@mitchpolygon mitchpolygon dismissed arnaubennassar’s stale review November 21, 2024 14:26

Made almost all of the changes, we can discuss further changes in a new PR -> goal is to get this one live and assess from the staging env.

@mitchpolygon mitchpolygon marked this pull request as ready for review November 21, 2024 14:26
@mitchpolygon mitchpolygon merged commit 152204c into main Nov 21, 2024
2 checks passed
@mitchpolygon mitchpolygon deleted the hosted/cdk-pp-docs-review-env branch November 21, 2024 14:28
@fukinbubba
Copy link

In having password issues. Had someone resolve most of it but seems it still isn't. I'm trying to work on it now but I'm by myself and need time. I don't want to run out of time again before I can get this resolved. Send me discord invite please

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

Successfully merging this pull request may close these issues.

5 participants