8000 Error in the introductory guides · Issue #1583 · cometbft/cometbft · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Error in the introductory guides #1583

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
Closed
lasarojc opened this issue Nov 8, 2023 · 2 comments · Fixed by #1603
Closed

Error in the introductory guides #1583

lasarojc opened this issue Nov 8, 2023 · 2 comments · Fixed by #1603
Assignees
@andynog
Labels
documentation Improvements or additions to documentation

Comments

@lasarojc
Copy link
Contributor
lasarojc commented Nov 8, 2023

Bug Report

The introductory go.md and go-builtin.md guides have a wrong example of prepareProposal, as noted [here] (#1384 (comment))

func (app *KVStoreApplication) PrepareProposal(_ context.Context, proposal *abcitypes.RequestPrepareProposal) (*abcitypes.ResponsePrepareProposal, error) {
   totalBytes := int64(0)
   txs := make([]byte, 0)

   for _, tx := range proposal.Txs {
     totalBytes += int64(len(tx))
     txs = append(txs, tx...)

       if totalBytes > int64(proposal.MaxTxBytes) {
         break
       }
     }

     return &abcitypes.ResponsePrepareProposal{Txs: proposal.Txs}, nil
 }

First, txs should be a slice of slices.
Second, the overflow check should be done before appending the transaction to txs.

The fix need to be back ported to 0.38 and 0.37.
@lasarojc lasarojc added bug Something isn't working documentation Improvements or additions to documentation labels Nov 8, 2023
@lasarojc lasarojc mentioned this issue Nov 8, 2023
Merged
@andynog andynog removed the bug Something isn't working label Nov 9, 2023
@andynog andynog self-assigned this Nov 9, 2023
@andynog
Copy link
Contributor
andynog commented Nov 9, 2023

Thanks @lasarojc for pointing this out. I'll take on this and fix the docs and backport it

@andynog andynog moved this from Todo to In Progress in CometBFT 2023 Nov 9, 2023
@andynog andynog moved this from In Progress to Todo in CometBFT 2023 Nov 9, 2023
@melekes melekes mentioned this issue Nov 14, 2023
Merged
3 tasks
@melekes
Copy link
Contributor
melekes commented Nov 14, 2023

I agree with @lasarojc here that it's an intro document and we shouldn't overwhelm devs with details. Simply relaying existing transactions without modifying them is enough, imho.

@github-project-automation github-project-automation bot moved this from Todo to Done in CometBFT 2023 Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@andynog andynog

Labels
documentation Improvements or additions to documentation
Projects
No open projects
CometBFT 2023
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

docs: various improvements cometbft/cometbft
3 participants
@melekes @lasarojc @andynog
0