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

[Access] SubscribeBlocks endpoint is missing #564

Conversation

UlyanaAndrukhiv
Copy link
Collaborator

@UlyanaAndrukhiv UlyanaAndrukhiv commented Feb 7, 2024

FLIP: onflow/flips#229

Description

This pull request :

  • implements missing SubscribeBlocks, SubscribeBlockHeaders and SubscribeBlockDigests methods from Access API interface
  • updates RegisterAccessAPIServer, StateStreamBackend and creating backend.Config according to new flow-go package version
  • resolves dependencies (removed flow-archive dependency & storage/remote package) according to PR [Remove flow-archive dependency & storage/remote package #524]
  • fixed tests (according to comment)

@turbolent
Copy link
Member

Nice! Is this also going to be ported to master?

@UlyanaAndrukhiv
Copy link
Collaborator Author

Hi, @tarakby,
I observed that you recently you had cleaned up the replace statements in flow-go ./integration and flow-go still uses the untagged version of flow-emulator from tarak/v0.58.1-0-with-crypto-type-fixes branch. To address this, I have prepared this PR that includes the necessary changes to this tarak/v0.58.1-0-with-crypto-type-fixes branch.

@tarakby
Copy link
Contributor

tarakby commented Feb 7, 2024

Hi @UlyanaAndrukhiv, thanks for the ping! Yes you are right, this non tagged branch is used by flow-go because it still uses Cadence prior to 1.0. Importing flow-emulator master or other recent tags causes Cadence 1.0 to be imported in flow-go which is not possible yet.
I agree with the idea to extend my branch, though am not maintaining the flow-emulator repo and don't know the context of your changes. I therefore let the other people review the changes and approve.

I also think your changes could be backported into master so they don't get lost once we're done with my branch.

@turbolent
Copy link
Member

turbolent commented Feb 7, 2024

So flow-go master uses Emulator's tarak/v0.58.1-0-with-crypto-type-fixes branch (https://github.com/onflow/flow-go/blob/master/integration/go.mod#L26).

The name of that branch looks like a temporary name. Maybe the Emulator repo should have a dedicated cadence-v0.42 branch?

@turbolent
Copy link
Member

cc @sideninja: This is one of the solutions I had mentioned re: EVM. Once we have this branch, maybe EVM related PRs that are already on master, like #515, can be backported, and that would unblock the EVM + Cadence v0.42 work?

@UlyanaAndrukhiv
Copy link
Collaborator Author

@sideninja, what are your thoughts on the discussion upper? Thank you in advance.

@sideninja
Copy link
Contributor

We should create a branch like @turbolent suggested that is pre-cadence 1.0. As a source branch, the Taraks' branch can be used. Then if there's something that is needed to unblock we can backport changes. However I think that for the purpose of flow-go EVM work it's better to use the updated feature/cadence branch since anyhow we are preparing for Crescendo.

@UlyanaAndrukhiv if you want you can create that yourself but if you need help or want me to create the base branch let me know.

@chasefleming
Copy link
Member

Heads up, I have created a branch off of v0.59.0 called release/v0 for #566 . This is so we can continue releasing before v.1.0.0. If you need something to go out before v1 this would be the branch to do it off of from here on out. Note that anything merged to this branch will also need to be cherry picked to master. cc @turbolent

@turbolent turbolent mentioned this pull request Feb 12, 2024
6 tasks
@UlyanaAndrukhiv
Copy link
Collaborator Author

I recreated a new PR #593 that targets the release/v0 branch

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.

5 participants