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

tendermint/0.33 / #196 followup issue #211

Closed
9 tasks done
liamsi opened this issue Apr 10, 2020 · 5 comments
Closed
9 tasks done

tendermint/0.33 / #196 followup issue #211

liamsi opened this issue Apr 10, 2020 · 5 comments
Assignees
Labels
go Compatibility with Go code

Comments

@liamsi
Copy link
Member

liamsi commented Apr 10, 2020

Collecting all outstanding discussions for full 0.33 compatibility and review comments from #196 and #199 and related issues here:


@ancazamfir re-tested this and it

Seems ok now

This was likely resolved as part of the serialization refactoring @greg-szabo did.


@liamsi liamsi mentioned this issue Apr 21, 2020
5 tasks
@ebuchman ebuchman added this to the v0.33 Compatibility milestone Jun 3, 2020
@ebuchman ebuchman added the go Compatibility with Go code label Jun 3, 2020
@liamsi
Copy link
Member Author

liamsi commented Jun 3, 2020

I think #196 (comment) is resolved. At least I would think this would be caught by the integration tests 🙏
Nah, NVM:

investigate why above isn't detected by the integration tests

@liamsi liamsi mentioned this issue Jun 3, 2020
5 tasks
@liamsi
Copy link
Member Author

liamsi commented Jun 3, 2020

tests for jsonrpc ID: #196 (comment)

This would become obsolete if we switched to using an jsonrpc library instead. Otherwise the tests I had in mind would look similar to: https://github.com/paritytech/jsonrpc/blob/ef92a481d76802da489f0f57dfdf4d459d7f7f76/core/src/types/id.rs#L16

@liamsi
Copy link
Member Author

liamsi commented Jun 3, 2020

Regarding the last point:

move Proof: #206 (review)

There is also a separate tracking issue: #207
Will comment there but tldr: the struct in abci is only there for serialzation. I think it isn't as displaced as it looks in the first place.

@liamsi liamsi mentioned this issue Jun 3, 2020
5 tasks
@liamsi
Copy link
Member Author

liamsi commented Jun 3, 2020

This issue can be closed when #297 and #298 are merged.

@liamsi
Copy link
Member Author

liamsi commented Jun 3, 2020

All comments addressed as far as I can see.

@liamsi liamsi closed this as completed Jun 3, 2020
@liamsi liamsi self-assigned this Jun 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Compatibility with Go code
Projects
None yet
Development

No branches or pull requests

2 participants