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

Added compatibility tracking #334

Merged
merged 11 commits into from
Nov 4, 2022
Merged

Added compatibility tracking #334

merged 11 commits into from
Nov 4, 2022

Conversation

dasconnor
Copy link
Contributor

Added a compatibility.json similar to the one being added in avalanchego. Will allow external apps to determine which version of avalanchego to use without needing to parse the readme. Also added a test to make sure the json parses.

This file will need to be updated with each new subnet-evm release.

Comment on lines 15 to 26
func TestCompatibility(t *testing.T) {
subevmVersion := "v0.4.0"
expectedRPCVersion := 17

compat, err := os.ReadFile("../compatibility.json")
assert.NoError(t, err)

var parsedCompat rpcChainCompatibility
err = json.Unmarshal(compat, &parsedCompat)
assert.NoError(t, err)
assert.Equal(t, expectedRPCVersion, parsedCompat.RPCChainVMProtocolVersion[subevmVersion])
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we move this test to plugin/evm/version_test.go and use the actual version and actual imported rpcchainvm.ProtocolVersion from AvalancheGo, so that if we mess this up for the latest version this. test will fail?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a test, for it to work tho you will need to remember to update the version in plugins/evm/version.go.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's the goal 😉

subevmVersion := "v0.4.0"
expectedRPCVersion := 17

compat, err := os.ReadFile("../compatibility.json")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

go:embed only works with files in a subdirectory of the test. I don't think we want to bury this json file in subdirectories, so I'm not going to make this change.

Copy link
Collaborator

@aaronbuchwald aaronbuchwald left a comment

Choose a reason for hiding this comment

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

Thanks for putting this up, could you make two small changes and then I think this should be good to merge and will make sure that we do not accidentally break this in the future.

Comment on lines 38 to 39
_, valueInJSON := parsedCompat.RPCChainVMProtocolVersion[Version]
assert.True(t, valueInJSON)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should assert that the actual value matches the rpcchainvm protocol version that we have imported from avalanchego as well.

Copy link
Collaborator

@aaronbuchwald aaronbuchwald left a comment

Choose a reason for hiding this comment

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

LGTM - thanks for putting this up 🙌

@@ -15,7 +15,7 @@ replace github.com/ava-labs/subnet-evm => ../..

require (
github.com/VictoriaMetrics/fastcache v1.10.0 // indirect
github.com/ava-labs/avalanchego v1.9.1 // indirect
github.com/ava-labs/avalanchego v1.9.2-rc.7 // indirect
Copy link
Contributor

Choose a reason for hiding this comment

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

remove rc

@patrick-ogrady patrick-ogrady mentioned this pull request Nov 4, 2022
@aaronbuchwald aaronbuchwald merged commit 55518f7 into master Nov 4, 2022
@aaronbuchwald aaronbuchwald deleted the addComposability branch November 4, 2022 22:55
arunraoshrap pushed a commit to neonmachine-shrapnel/subnet-evm that referenced this pull request Nov 8, 2022
* Added compatibility tracking

* Added version moved test

* Cleaned up test

* Bump avalanchego version and nits

* go mod tidy cmd/simulator

* Mark trie clean jouranl test as flaky

* Bump avalanchego to v1.9.2

* Bump anr version in scripts/versions.sh

* Bump anr version in go.mod

* go mod tidy cmd/simulator

Co-authored-by: Aaron Buchwald <[email protected]>
ceyonur pushed a commit that referenced this pull request Sep 17, 2023
* drop outbound gossip requests for non validators

* nit

* nit
aaronbuchwald pushed a commit that referenced this pull request Sep 19, 2023
* Drop outbound gossip requests for non-validators (#334)

* drop outbound gossip requests for non validators

* nit

* nit

* sync changes

---------

Co-authored-by: Joshua Kim <[email protected]>
aaronbuchwald pushed a commit that referenced this pull request Sep 19, 2023
* Add SDK Router message handling (#316)

Co-authored-by: Stephen Buttolph <[email protected]>

* Fix hanging requests after Shutdown (#326)

* fix requests hanging after shutdown

* fix build

---------

Signed-off-by: Stephen Buttolph <[email protected]>
Co-authored-by: Stephen Buttolph <[email protected]>

* Update to 1.10.10-rc.2 (#328)

* update to avalanchego 1.10.10-rc.2

* nits

* nit

* Add P2P SDK Pull Gossip (#318)

* add batchsize

* sync changes

* Drop outbound gossip for non vdrs (#862)

* Drop outbound gossip requests for non-validators (#334)

* drop outbound gossip requests for non validators

* nit

* nit

* sync changes

---------

Co-authored-by: Joshua Kim <[email protected]>

---------

Co-authored-by: Joshua Kim <[email protected]>
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.

3 participants