Faddat/tweaks encountered along the way

[faddat]

context

And I might be wrong about this second part, but looking at where this value is used https://github.com/osmosis-labs/cometbft/blob/490dd752eb8a1a934542f70fdc8960594e06973b/blocksync/pool.go#L264-L268, it seems like because we are fetching blocks very far out, we just error and disregard them, so its completely wasted cycles AFAICT.

[pool.go](https://github.com/osmosis-labs/cometbft/blob/490dd752eb8a1a934542f70fdc8960594e06973b/blocksync/pool.go)
        if diff > maxDiffBetweenCurrentAndReceivedBlockHeight {
            pool.sendError(errors.New("peer sent us a block we didn't expect with a height too far ahead/behind"), peerID)
        }
        return
    }
Show more
<https://github.com/[osmosis-labs/cometbft](https://github.com/osmosis-labs/cometbft)|osmosis-labs/cometbft>osmosis-labs/cometbft | Added by https://notionalgroup.slack.com/services/B028F6EMYQM
21 replies


Jacob Gadikian
:spiral_calendar_pad:  [7 hours ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708862641221399?thread_ts=1708749103.552179&cid=CUB7CESLX)
Depending on where this is called, it could cause disconnection from that peer.


Jacob Gadikian
:spiral_calendar_pad:  [7 hours ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708862653674439?thread_ts=1708749103.552179&cid=CUB7CESLX)
When a peer accumulates errors it is put into a naughty lists, and disconnected from


Jacob Gadikian
:spiral_calendar_pad:  [6 hours ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708863032069109?thread_ts=1708749103.552179&cid=CUB7CESLX)
[@adam](https://notionalgroup.slack.com/team/U02RS0RBDJ6)
 is the cometbft fork you're linking to above compatible with the current osmosis release?


Jacob Gadikian
:spiral_calendar_pad:  [6 hours ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708863050326239?thread_ts=1708749103.552179&cid=CUB7CESLX)
becasue I can try to cherry pick the pool routine changes into it, or such.  I think it will also reduce ram/cpu use


adam
  [1 hour ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708880922051329?thread_ts=1708749103.552179&cid=CUB7CESLX)
Yeah it is compatible. Two changes should be made
Only fetch the next x blocks and respect this limit
when fetching blocks, maybe 50 blocks ahead of you, request from 3 or more peers and take the one that gets returned first
(edited)


adam
  [1 hour ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708880964930639?thread_ts=1708749103.552179&cid=CUB7CESLX)
This will make a large difference, paired with async pruning. These two problems fixes there will essentially never be a time when a node is syncing that it pauses for a couple of seconds
:100:
1



adam
  [1 hour ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708880989173939?thread_ts=1708749103.552179&cid=CUB7CESLX)
The pause is always one of these two issues, peer starving a block from you or your node is pruning


Jacob Gadikian
:spiral_calendar_pad:  [1 hour ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708881934727129?thread_ts=1708749103.552179&cid=CUB7CESLX)
yeah validators call "pauses for a couple of seconds" "the persistent peer issue"


Jacob Gadikian
:spiral_calendar_pad:  [1 hour ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708881939968479?thread_ts=1708749103.552179&cid=CUB7CESLX)
idk if you ever tried just having seeds


adam
  [1 hour ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708882640176529?thread_ts=1708749103.552179&cid=CUB7CESLX)
It still won’t help given we only fetch one block from one peer


adam
  [1 hour ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708882723949479?thread_ts=1708749103.552179&cid=CUB7CESLX)
So there is always a single point of failure. That paired with the fact that the settings only allow for a peer to have 20 pending requests, you are forced to have a breadth of peers you are requesting from, which eventually causes you to get stuck on one


Jacob Gadikian
:spiral_calendar_pad:  [1 hour ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708882782155919?thread_ts=1708749103.552179&cid=CUB7CESLX)
Huh well this will be fun.


Jacob Gadikian
:spiral_calendar_pad:  [1 hour ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708882821903249?thread_ts=1708749103.552179&cid=CUB7CESLX)
One of my earliest issues on the tenderment repository was actually the speed of block synchronization.  I was comparing to chains written in Dan larimers graphene thingy (very very boost libs and c++) and... Yeah we got completely smoked in the comparison


adam
  [1 hour ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708882944395199?thread_ts=1708749103.552179&cid=CUB7CESLX)
I think these two changes alone get us generally where we need to be, with future research on further improvements still being needed after
https://osmosis-network.slack.com/archives/CUB7CESLX/p1708880922051329?thread_ts=1708749103.552179&channel=CUB7CESLX&message_ts=1708880922.051329




adam
Yeah it is compatible. Two changes should be made
Only fetch the next x blocks and respect this limit
when fetching blocking, idk maybe 50 blocks ahead of you, request from 2 or 3 peers and take the one that gets returned first
(edited)
[From a thread in shared-tendermint](https://osmosis-network.slack.com/archives/CUB7CESLX/p1708880922051329?thread_ts=1708749103.552179&amp;channel=CUB7CESLX&amp;message_ts=1708880922.051329) | [Today at 7:08 PM](https://osmosis-network.slack.com/archives/CUB7CESLX/p1708880922051329?thread_ts=1708749103.552179&amp;channel=CUB7CESLX&amp;message_ts=1708880922.051329) | [View reply](https://osmosis-network.slack.com/archives/CUB7CESLX/p1708880922051329?thread_ts=1708749103.552179&amp;channel=CUB7CESLX&amp;message_ts=1708880922.051329)
:+1:
1



Jacob Gadikian
:spiral_calendar_pad:  [1 hour ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708883054305239?thread_ts=1708749103.552179&cid=CUB7CESLX)
Do you prefer this to be made against main and then backported to 37 or against 37 and then forward ported to main?


adam
  [1 hour ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708883143773779?thread_ts=1708749103.552179&cid=CUB7CESLX)
Osmosis likely will be on 37 for the foreseeable future until sdk 50 issues get worked out via dydx so selfishly I say the latter, but former is also fine!
:+1:
1



Jacob Gadikian
:spiral_calendar_pad:  [1 hour ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708883244539519?thread_ts=1708749103.552179&cid=CUB7CESLX)
Okay, you know it'll probably be easier to do 37, there won't be any concerns from vote extensions.  I tried to backport my pr, but it got pretty messy.


Jacob Gadikian
:spiral_calendar_pad:  [1 hour ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708883272072309?thread_ts=1708749103.552179&cid=CUB7CESLX)
Oh by the way I think I did succeed in getting you metrics for stuff like block size, by backporting.


adam
  [44 minutes ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708883635808589?thread_ts=1708749103.552179&cid=CUB7CESLX)
oh i was just able to print them in a println, the largest i saw was 195000bytes but would like to see if you saw differently


adam
  [44 minutes ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708883658999979?thread_ts=1708749103.552179&cid=CUB7CESLX)
(you can print the block.Size when you fetch from a peer and i just read from those)


Jacob Gadikian
:spiral_calendar_pad:  [33 minutes ago](https://notionalgroup.slack.com/archives/CUB7CESLX/p1708884344214839?thread_ts=1708749103.552179&cid=CUB7CESLX)
so, it gets subjective really around the tx results --  if it is just the proto stuff, I think your number sounds totally correct

---

PR checklist

• Tests written/updated
• Changelog entry added in .changelog (we use unclog to manage our changelog)
• Updated relevant documentation (docs/ or spec/) and code comments

[github-actions]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.