Skip to content

Conversation

@Reisen
Copy link
Contributor

@Reisen Reisen commented Apr 22, 2024

This PR refactors the Adapter to remove the running service component. The goal behind this is to remove the channel communication between the adapter and other components, instead, Adapter now acts as an API. The Adapter API currently is a single nameless trait with all functionality combined, this change will allow splitting that API out into relevant components to better isolate functionality. For now though, this just moves the entire API into a temporary trait called AdapterApi, in future PR's, adapter/ will become state/ and the relevant functionality will be broken out.

Apart from the AdapterApi and channel removal, all other changes are mechanical, just propagating the change through the codebase.

Some changes that occurred during this refactoring:

  • Tests from rpc.rs are removed, as they don't actually test anything meaningful (just whether a send/recv over a channel with hardcoded values works)
  • A temporary notifier service is in .../adapter/api.rs to replace the only meaningful runtime component of adapter.
  • An abstract S parameter is added anywhere Adapter used to be relied on.
  • The counter logic mildly changed as it is now an atomic, which makes the ++increment behaviour increment++
  • The AdapterApi is now strongly typed, so the rpc.rs API now does proper verification of accounts in the right place.
  • Non-trait methods factored out of AdapterApi

@Reisen Reisen requested a review from ali-behjati April 22, 2024 06:47
@Reisen Reisen force-pushed the push-qywryorxvyvr branch from 48b59c1 to 9d3cb67 Compare April 22, 2024 06:50
tokio::task::spawn(serve).await.map_err(|e| e.into())
}

#[cfg(test)]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually there is some value to these, I will bring these back but in a slightly different form.

Copy link
Collaborator

Choose a reason for hiding this comment

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

i think the adaptor tests are a bit useless but this is good.

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