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

1606 node clients replaces merge #1610

Merged
merged 19 commits into from
Jun 12, 2024
Merged

1606 node clients replaces merge #1610

merged 19 commits into from
Jun 12, 2024

Conversation

m4sterbunny
Copy link
Contributor

Instances referring to "The Merge" linked to Ethereum docs
The Merge page >> Node clients
Small updates to affected pages

@m4sterbunny
docs(update):issue 1606
1a3f42f 
Signed-off-by: m4sterbunny <harrie.bickle@consensys.net>
@m4sterbunny
proofs pages affected by 1606
84d571d 
Signed-off-by: m4sterbunny <harrie.bickle@consensys.net>
@m4sterbunny m4sterbunny requested a review from a team as a code owner June 7, 2024 08:17
@vercel Vercel
Copy link

vercel bot commented Jun 7, 2024 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
besu-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 12, 2024 8:27am

@m4sterbunny
proofs pages affected by 1606
761d5a9 
Signed-off-by: m4sterbunny <harrie.bickle@consensys.net>
@m4sterbunny
proofs pages affected by 1606
92084e8 
Signed-off-by: m4sterbunny <harrie.bickle@consensys.net>
@m4sterbunny
proofs pages affected by 1606
876ff69 
Signed-off-by: m4sterbunny <harrie.bickle@consensys.net>
@m4sterbunny
fixes flawed redirect for 1606
4c532a0 
Signed-off-by: m4sterbunny <harrie.bickle@consensys.net>
@joaniefromtheblock
test redirect
bcdbdbd 
Signed-off-by: Joan E <153745173+joaniefromtheblock@users.noreply.github.com>
@m4sterbunny
temp fix to allow redirect verification
bd31909 
Signed-off-by: m4sterbunny <harrie.bickle@consensys.net>
bgravenorst
bgravenorst reviewed Jun 9, 2024
View reviewed changes
Copy link
Contributor

@bgravenorst bgravenorst left a comment

Choose a reason for hiding this comment

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

Some feedback. I'd suggest a technical review since there's additional content here.

Also, we generally use a 100 character line limit: https://docs-template.consensys.io/contribute/format-markdown#column-limit

docs/public-networks/concepts/node-clients.md Outdated Show resolved Hide resolved
docs/public-networks/concepts/node-clients.md Show resolved Hide resolved
@m4sterbunny
better (not total) character limit adherence
1f8c380 
Signed-off-by: m4sterbunny <harrie.bickle@consensys.net>
@m4sterbunny
better (not total) character limit adherence
e9a2c7f 
Signed-off-by: m4sterbunny <harrie.bickle@consensys.net>
@m4sterbunny
Update node-clients.md
fcdb2d5 
proof

Signed-off-by: m4sterbunny <harrie.bickle@consensys.net>
@m4sterbunny
Update node-clients.md
e0380ca 
as per discussion with Fabio

Signed-off-by: m4sterbunny <harrie.bickle@consensys.net>
jflo
jflo approved these changes Jun 11, 2024
View reviewed changes
Copy link
Contributor

@jflo jflo left a comment

Choose a reason for hiding this comment

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

Minor suggestions, looks fine otherwise.

docs/public-networks/concepts/node-clients.md Outdated Show resolved Hide resolved
docs/public-networks/concepts/node-clients.md Outdated Show resolved Hide resolved
@m4sterbunny
Update node-clients.md
d36263e 
responds to tech review

Signed-off-by: m4sterbunny <harrie.bickle@consensys.net>
bgravenorst
bgravenorst approved these changes Jun 11, 2024
View reviewed changes
Copy link
Contributor

@bgravenorst bgravenorst left a comment

Choose a reason for hiding this comment

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

Provided some additional (minor) feedback.

docs/public-networks/concepts/node-clients.md Outdated Show resolved Hide resolved
docs/public-networks/concepts/node-clients.md Outdated Show resolved Hide resolved
@m4sterbunny
responds to review
a62f5be 
Signed-off-by: m4sterbunny <harrie.bickle@consensys.net>
@m4sterbunny
Add files via upload
a771b95 
Signed-off-by: m4sterbunny <harrie.bickle@consensys.net>
@m4sterbunny
updates image to rmv bi direction
15a0989 
Signed-off-by: m4sterbunny <harrie.bickle@consensys.net>
@m4sterbunny
Rename
f94af06 
Execution-Consensus-Client-EngineAPI.png to Execution-Consensus-Clients-EngineAPI.png

Signed-off-by: m4sterbunny <harrie.bickle@consensys.net>
This was referenced Jun 12, 2024
Closed
Closed
@m4sterbunny
Copy link
Contributor Author

Not merging yet -- want to verify that image update is def fine.
Image now clarifies that the relationship btwn consensus client and execution client is one way via the Engine API

But, that leaves open the Q: how does the consensus client get "the validated data from the execution client"?

@m4sterbunny
Copy link
Contributor Author

OK -- so going to merge -- Execution does emit messages back to Consensus, but apparently the single headed arrow show direction of API requests as standard >> whereas sequence diagram would specify the response

@m4sterbunny m4sterbunny merged commit 0edd6e8 into hyperledger:main Jun 12, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bgravenorst bgravenorst bgravenorst approved these changes

@jflo jflo jflo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@m4sterbunny @jflo @bgravenorst @joaniefromtheblock