Skip to content

Conversation

@zsfelfoldi
Copy link
Contributor

This PR allows tx unindexing in les/4 light server mode.
A few more upgrades might be added to les/4 but the goal is to keep the number of changes low while satisfying the requirements stated in #21876
Most of the upgrades needed for incentivization (including UDP communication) should go into les/5.

Features that might be added to the les/4 upgrade (up for debate):

  • forkID; see les, core: introduce les4 #20227
  • reduced minimum capacity in order to serve more clients
  • request/reply metainfo channel; see zsfelfoldi@48c7ec9 (parts of this commit are related to lespay request serving which should definitely go into the next upgrade)

Note: the metainfo channel upgrade is needed for incentivization and therefore could also go into les/5. The reason I added it here is because it can be added separately from everything else and it would make the next upgrade easier (less changes at once). I think with the rest of the changes postponed this upgrade would still be relatively low risk if we add the metainfo and it would help my progress but if there is a good reason then we can leave it out.

les/peer.go Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Can we put these number in the protocol file and use the constants? It's a bit weird to hardcode the number here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Member

Choose a reason for hiding this comment

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

Sorry one more nitpicks, can we replace the 1 with a constant? It's better to define in the protocol that 1 means disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Member

@rjl493456442 rjl493456442 left a comment

Choose a reason for hiding this comment

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

Just one nitpick ,otherwise LGTM. Also test it, it works.

les/peer.go Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Sorry one more nitpicks, can we replace the 1 with a constant? It's better to define in the protocol that 1 means disabled.

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.

2 participants