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

Refactor vm.Config out of vm.Controller #1146

Merged
merged 5 commits into from
Jul 17, 2024
Merged

Refactor vm.Config out of vm.Controller #1146

merged 5 commits into from
Jul 17, 2024

Conversation

joshua-kim
Copy link
Contributor

Controller implementations are supposed to parse the provided config bytes upon initialization into a hypersdk config. This PR parses the config in the hypersdk, and supplies the remaining bytes to the controller to reduce boilerplate code in the implementation.

}
func (c *Config) GetStateSyncServerDelay() time.Duration { return c.StateSyncServerDelay }
func (c *Config) GetStreamingBacklogSize() int { return c.StreamingBacklogSize }
func (c *Config) GetContinuousProfilerConfig() *profiler.Config {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some of these config values weren't even used... the profiler is interesting though so we could keep this if we wanted to provide it out-of-the-box (although it isn't used right now).

Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
@joshua-kim joshua-kim marked this pull request as ready for review July 17, 2024 07:07
iFrostizz
iFrostizz previously approved these changes Jul 17, 2024
Copy link
Contributor

@iFrostizz iFrostizz left a comment

Choose a reason for hiding this comment

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

It's clear that most of these functions were not used, but couldn't they be part of a public API that all VM implementers would expect these to comply with ?

@joshua-kim
Copy link
Contributor Author

Chatted offline. I think it's fine for the VM to not know about the hypersdk config since it holds values that only hypersdk will ever care about. If it holds something the VM needs we would expect that to be in the VM config.

Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
@joshua-kim joshua-kim enabled auto-merge (squash) July 17, 2024 16:33
@joshua-kim joshua-kim merged commit cd8c3ef into main Jul 17, 2024
20 checks passed
@joshua-kim joshua-kim deleted the refactor-configs branch July 17, 2024 16:52
iFrostizz pushed a commit that referenced this pull request Jul 18, 2024
* refactor hypersdk config out of controllers

Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
aaronbuchwald added a commit that referenced this pull request Jul 23, 2024
* implement action test suite helper

* add morpheusvm action tests

* add check for contains error for dynamic errors

* remove morpheusvm test, simplify suite

* add morpheusvm parametrized tests

* add licence

* check err

* cleanup

* remove `Actor` usage and implement `InMemoryStore`

* remove unused ctx

* add review suggestions

* Update chaintesting/action_test_helpers.go

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>
Signed-off-by: Franfran <51274081+iFrostizz@users.noreply.github.com>

* Update chaintesting/action_test_helpers.go

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>
Signed-off-by: Franfran <51274081+iFrostizz@users.noreply.github.com>

* add tests, set actor, add assertion

* Updated Token Example (#1142)

* token example

* StateKey -> StateKeys

* nits

* update asserts and unwraps

* Expose state functions from context (#1153)

* use `WindowSize` constant (#1155)

* update readme to point to an existing tokenvm action (#1049)

* update readme to point to an existing tokenvm action

* update readme: change action name

---------

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>

* Refactor `vm.Config` out of `vm.Controller` (#1146)

* refactor hypersdk config out of controllers

Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>

* remove unused parallelism key from test configs (#1158)

Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>

* Remove `key_create` endpoint and use `Address` everywhere (#1121)

* pass directly actor address to simulator

* remove unused params

* Conditionally compile the `Address` helpers

* add test-utils flag

* Don't add unnecessary dependency (#1122)

* cleanup

* read `Actor` directly

* remove support for `create_key` endpoint and replace everything by
`Address` instances

* remove unused params

* remove `Key` `Param` (#1131)

* remove `Address::from_str`

---------

Co-authored-by: Richard Pringle <richard.pringle@avalabs.org>

* rename to chaintest, slice of `ActionTest`, remove suite

* rename to `InMemoryStore` to export type

* Update examples/morpheusvm/actions/transfer_test.go

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>
Signed-off-by: Franfran <51274081+iFrostizz@users.noreply.github.com>

* add sender post-balance assertion

* pass in `*testing.T` to the `Assertion` func pointer

* pass context from `Run`, handle error signal on done

* Context plumbing in `ActionTest` suite (#1170)

* refactor: run tests individually to avoid `Context` pollution

* refactor: pipe `Contexts` where possible

The functions that are immediately run at declaration don't have a natural means of accepting a `Context` so can be considered the same as the `TestXXX()` entrypoint function.

* write helper function

* Update chaintest/action_test_helpers.go

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>
Signed-off-by: Franfran <51274081+iFrostizz@users.noreply.github.com>

* Update examples/morpheusvm/actions/transfer_test.go

Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>
Signed-off-by: Franfran <51274081+iFrostizz@users.noreply.github.com>

---------

Signed-off-by: Franfran <51274081+iFrostizz@users.noreply.github.com>
Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Co-authored-by: aaronbuchwald <aaron.buchwald56@gmail.com>
Co-authored-by: Sam Liokumovich <65994425+samliok@users.noreply.github.com>
Co-authored-by: Richard Pringle <richard.pringle@avalabs.org>
Co-authored-by: nathan haim <haim.nathan@icloud.com>
Co-authored-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Co-authored-by: Arran Schlosberg <519948+ARR4N@users.noreply.github.com>
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.

None yet

3 participants