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

Summary of Upcoming Breaking Changes #1568

Closed
15 of 19 tasks
ebuchman opened this issue May 14, 2018 · 13 comments
Closed
15 of 19 tasks

Summary of Upcoming Breaking Changes #1568

ebuchman opened this issue May 14, 2018 · 13 comments
Labels
C:abci Component: Application Blockchain Interface C:consensus Component: Consensus C:p2p Component: P2P pkg
Milestone

Comments

@ebuchman
Copy link
Contributor

ebuchman commented May 14, 2018

Proposed summary of impending breaking changes.

The goal is to get as many of the breaking data structure changes into the next breaking release as possible :)

@ebuchman
Copy link
Contributor Author

ebuchman commented May 14, 2018

Note also from #1502:

@ebuchman ebuchman added C:abci Component: Application Blockchain Interface C:p2p Component: P2P pkg C:consensus Component: Consensus user labels May 14, 2018
@ebuchman
Copy link
Contributor Author

Another point still up for debate is what to do about pubkeys in the ABCI (ie. whether or not to involve Amino) - #1524

@ebuchman
Copy link
Contributor Author

Maybe change LastBlockID to LastBlockHash in the header? #1605

@ebuchman
Copy link
Contributor Author

ebuchman commented May 22, 2018

Ideally the encoding of the header would be compatible with a protobuf encoding - #1605 (comment)

Would be nice to not build these SimpleMap structures ... just noticed the discrepency between how we do it for Header and for ConsensusParams ...

@cwgoes
Copy link
Contributor

cwgoes commented May 23, 2018

Can we include ABCI evidence updates - tendermint/abci#243 - and the associated Tendermint changes - #1582 - in this changeset?

edit: Also inclusion of both present and absent validators, and all validator data, in BeginBlock per SDK meeting (with future optimizations possible).

@jaekwon
Copy link
Contributor

jaekwon commented May 29, 2018

While we can have both present and absent validators in BeginBlock, it should be noted that validator updates apply to the next validator set, not the one received via BeginBlock. This means that ABCI developers should take note to persist the validator set somewhere...

Which kind of defeats the point of using addresses vs indices, imo, since it's easy enough to sort a validator set once you have one. But that's fine. I just prefer not to send both validator sets to the app via ABCI, because that's "helping" the ABCI developer a little too much to the detriment of performance.

@ebuchman
Copy link
Contributor Author

ebuchman commented May 30, 2018

More changes discussed:

  • version in header
  • merkle tree of tags for efficient proofs of inclusion/exclusion of tags in a block (rather than putting the tags in the result merkle tree) - see Add events to results hash #1007

@ebuchman
Copy link
Contributor Author

Which kind of defeats the point of using addresses vs indices, imo, since it's easy enough to sort a validator set once you have one. But that's fine. I just prefer not to send both validator sets to the app via ABCI, because that's "helping" the ABCI developer a little too much to the detriment of performance.

This might turn out to be really unfortunate for devs ... hmm

@ebuchman ebuchman mentioned this issue Jun 8, 2018
@ebuchman
Copy link
Contributor Author

ebuchman commented Jun 8, 2018

Summary of state of this:

ABCI

  • Alot of this was released in v0.11
  • We still need to:
    • make the Header match the Tendermint header
    • dont send the pubkey in RequestBeginBlock (see updated ADR Bucky/adrs #1711)

Everything else

Everything else is basically in open PRs.

More

The last thing to work on is how we deal with versioning to facilitate upgrades - both on the blockchain datastructures and in the p2p layer.

I think we need to introduce versions for both these, but not exactly sure how. We should prepare an ADR for how to do this.

@xla xla added this to the launch milestone Jun 14, 2018
@ebuchman ebuchman added this to Planned in current iteration via automation Jul 23, 2018
@ebuchman
Copy link
Contributor Author

Everything in here is either complete or summarized in open issues that are well tagged.

current iteration automation moved this from Planned to Done Jul 27, 2018
@ttmc
Copy link

ttmc commented Sep 8, 2018

@ebuchman I realize you closed this issue, but we were using it to get a sense of how many breaking changes we can expect in the future. Could you check the boxes beside the things that are now done and merged? It would also be helpful to check the boxes beside the things that won't ever be done.

@ebuchman
Copy link
Contributor Author

ebuchman commented Sep 9, 2018

Updated. We're now using the "breaking" tag: https://github.com/tendermint/tendermint/issues?q=is%3Aopen+is%3Aissue+label%3Abreaking

@ttmc
Copy link

ttmc commented Sep 9, 2018

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:abci Component: Application Blockchain Interface C:consensus Component: Consensus C:p2p Component: P2P pkg
Projects
No open projects
Development

No branches or pull requests

5 participants