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

[Testing, Tooling] feat: implement in-memory networks #290

Closed
wants to merge 94 commits into from

Conversation

bryanchriswhite
Copy link
Contributor

@bryanchriswhite bryanchriswhite commented Dec 21, 2023

Summary

Human Summary

Adds implementations for the InMemoryCosmosNetwork interface from #289:

  • BaseInMemoryCosmosNetwork
    "Abstract" implementation intended to be embedded in "concrete" implementations. It implements the following interface members:

    • #GetClientCtx()
    • #GetNetworkConfig()
    • #GetNetwork()

    Additionally, it includes the following convenience methods:

    • #InitializeDefaults()
    • #GetLastAccountSequenceNumber()
    • #NextAccountSequenceNumber()
  • inMemoryNetworkWithSessions()
    Implementation where #Start() creates InMemoryNetworkConfig#NumSuppliers number of genesis suppliers and #NumSuppliers * #AppToSupplierRatio number of applications where #AppToSupplierRatio number of applications will match one service with each supplier (in distinct pairings). It also creates corresponding on-chain accounts (auth module) for each genesis actor. It exposes #CreateClaims() and #CreateProofs() methods for each session * InMemoryNetworkConfig#NumSession number of session.

  • inMemoryNetworkWithGateways()
    Implementation where #Start() creates InMemoryNetworkConfig#NumApplications and #NumGateways number of genesis applications and gateways, respectively. It also creates corresponding on-chain accounts (auth module) for each genesis actor.

Issue

Type of change

Select one or more:

  • New feature, functionality or library
  • Bug fix
  • Code health or cleanup
  • Documentation
  • Other (specify)

Testing

  • Run all unit tests: make go_develop_and_test
  • Run E2E tests locally: make test_e2e
  • Run E2E tests on DevNet: Add the devnet-test-e2e label to the PR. This is VERY expensive, only do it after all the reviews are complete.

Sanity Checklist

  • I have tested my changes using the available tooling
  • I have performed a self-review of my own code
  • I have commented my code, updated documentation and left TODOs throughout the codebase

@bryanchriswhite bryanchriswhite added testing Test (or test utils) additions, fixes, improvements or other tooling Tooling - CLI, scripts, helpers, off-chain, etc... labels Dec 21, 2023
@bryanchriswhite bryanchriswhite added this to the Shannon TestNet milestone Dec 21, 2023
@bryanchriswhite bryanchriswhite self-assigned this Dec 21, 2023
@bryanchriswhite bryanchriswhite force-pushed the issues/141/feat/in-memory-network branch 2 times, most recently from 11c019d to f03177e Compare December 22, 2023 12:28
bryanchriswhite and others added 14 commits December 22, 2023 14:36
Co-authored-by: harry <[email protected]>
Co-authored-by: Daniel Olshansky <[email protected]>
Co-authored-by: Daniel Olshansky <[email protected]>
Co-authored-by: harry <[email protected]>
…im-proof

* pokt/main:
  Fix bug introduced by #252 where genesis file was no longer being copied to the right location
  [Docs] Introduce Docusaurus documentation (#252)
  [Cleanup] Centralzie websocket url -> endpoint changes (#272)
  refactor: `NewMinedRelay` to shared testutil (#262)
  fix: PR template typo 2 (#269)
  [Testing] Unit tests, E2E tests, logging and other Fixes / "Touchups" (#253)
  [SDK] feat: Have distinct JSON-RPC and gRPC urls (#261)
…or/supplier-keys

* issues/141/refactor/claim-proof:
  chore: review feedback improvements
  chore: review feedback improvements
  Fix bug introduced by #252 where genesis file was no longer being copied to the right location
  [Docs] Introduce Docusaurus documentation (#252)
  [Cleanup] Centralzie websocket url -> endpoint changes (#272)
  refactor: `NewMinedRelay` to shared testutil (#262)
  fix: PR template typo 2 (#269)
  [Testing] Unit tests, E2E tests, logging and other Fixes / "Touchups" (#253)
  [SDK] feat: Have distinct JSON-RPC and gRPC urls (#261)
…ctor/supplier-errors

* issues/141/refactor/supplier-keys:
  chore: review feedback improvements
  chore: review feedback improvements
  Fix bug introduced by #252 where genesis file was no longer being copied to the right location
  [Docs] Introduce Docusaurus documentation (#252)
  [Cleanup] Centralzie websocket url -> endpoint changes (#272)
  refactor: `NewMinedRelay` to shared testutil (#262)
  fix: PR template typo 2 (#269)
  [Testing] Unit tests, E2E tests, logging and other Fixes / "Touchups" (#253)
  [SDK] feat: Have distinct JSON-RPC and gRPC urls (#261)
@bryanchriswhite bryanchriswhite changed the base branch from issues/141/prep/in-memory-network to main January 10, 2024 14:09
Copy link
Contributor

@h5law h5law left a comment

Choose a reason for hiding this comment

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

Overall this is such an amazing PR! Love the attention to detail, bumped a few things (I think) got overlooked but really 11/10 this is great work.

testutil/network/basenet/accounts.go Show resolved Hide resolved
testutil/network/basenet/accounts.go Outdated Show resolved Hide resolved
testutil/network/basenet/network.go Show resolved Hide resolved
testutil/network/gatewaynet/network.go Show resolved Hide resolved
testutil/network/gatewaynet/network.go Show resolved Hide resolved
testutil/network/gatewaynet/network.go Show resolved Hide resolved
testutil/network/sessionnet/network.go Show resolved Hide resolved
testutil/network/genesis.go Outdated Show resolved Hide resolved

// cliEncodeSessionHeader encodes the given session header as a base64-encoded
// string.
func cliEncodeSessionHeader(t *testing.T, sessionHeader *sessiontypes.SessionHeader) string {
Copy link
Member

Choose a reason for hiding this comment

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

Both optional

  1. Do we need cli in the method header?
  2. Thoughts on moving this into the session module types with the sessionHeader as a receiver?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is only used for CLI testing purposes. I don't think it would be reused anywhere.

testutil/network/sessionnet/sessions.go Outdated Show resolved Hide resolved
testutil/network/sessionnet/sessions.go Outdated Show resolved Hide resolved
testutil/network/sessionnet/sessions.go Outdated Show resolved Hide resolved
testutil/network/sessionnet/sessions.go Outdated Show resolved Hide resolved
testutil/network/sessionnet/claims.go Show resolved Hide resolved
testutil/network/sessionnet/claims.go Outdated Show resolved Hide resolved
require.NoError(t, err)
require.Equal(t, float64(0), responseJson["code"], "code is not 0 in the response: %v", responseJson)

// TODO_TECHDEBT: Forward the actual claim in the response once the response is updated to return it.
Copy link
Member

Choose a reason for hiding this comment

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

I don't fully understand this TODO given that we're returning it below. Can you expand?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated:

	// TODO_TECHDEBT: Forward the claim in the response, as opposed to constructing
	// an equivalent one here once the response and respective message handler is
	// updated to include it.

testutil/network/basenet/accounts.go Show resolved Hide resolved
testutil/network/basenet/accounts.go Outdated Show resolved Hide resolved
Copy link
Contributor

@h5law h5law left a comment

Choose a reason for hiding this comment

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

Great work, amazing - love to see how its coming together. Left a few comments, once they are done I think this one is good to go, lets get it in!

testutil/network/basenet/accounts.go Outdated Show resolved Hide resolved
testutil/network/basenet/accounts.go Outdated Show resolved Hide resolved
testutil/network/basenet/accounts.go Outdated Show resolved Hide resolved
testutil/network/basenet/accounts.go Show resolved Hide resolved
testutil/network/basenet/accounts.go Show resolved Hide resolved
testutil/network/basenet/accounts.go Outdated Show resolved Hide resolved
testutil/network/sessionnet/genesis.go Outdated Show resolved Hide resolved
@Olshansk
Copy link
Member

@h5law Just posting here for posterity, but let's leave this until after the v0.50 migration.

We may be leverage some of the newer patterns (example) as we integrate it with our specific business logic.

Also spoke about it with @bryanchriswhite as well

@bryanchriswhite
Copy link
Contributor Author

bryanchriswhite commented Jan 22, 2024

@h5law Just posting here for posterity, but let's leave this until after the v0.50 migration.

We may be leverage some of the newer patterns (example) as we integrate it with our specific business logic.

Also spoke about it with @bryanchriswhite as well

@Olshansk I would definitely hold off on #307 but I think we can push this through as reviewers are able to allocate bandwidth to it.

I think it represents a significant improvement from the existing tooling and can start to be used independent of the refactoring of existing integration tests.

@Olshansk
Copy link
Member

Discussed offline with @bryanchriswhite. Holding off on this until after the migration

@bryanchriswhite
Copy link
Contributor Author

Closing, superseded by #412 & #417.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testing Test (or test utils) additions, fixes, improvements or other tooling Tooling - CLI, scripts, helpers, off-chain, etc...
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants