soroban-rpc: Remove dependency on Horizon #431

2opremio · 2023-02-23T15:43:42Z

Fixes #430

2opremio · 2023-02-23T15:53:19Z

@sreuland @tamirms I am not sure how to handle the different workflows (build-latest.yml build-soroban-dev.yml build-testing.yml) It seems that latest is using 0.4, soroban-dev is using 0.6.1and testing is using 0.4

However, the removal of the Horizon dependency only happened in main. Maybe we should wait until all workflows surpass a version with Horizon removed?

common/soroban-rpc/bin/start

sreuland · 2023-02-23T20:02:20Z

@sreuland @tamirms I am not sure how to handle the different workflows (build-latest.yml build-soroban-dev.yml build-testing.yml) It seems that latest is using 0.4, soroban-dev is using 0.6.1and testing is using 0.4

However, the removal of the Horizon dependency only happened in main. Maybe we should wait until all workflows surpass a version with Horizon removed?

Ok, yes, I think for now, can continue to update just the workflows with soroban-tools ref where it's possible to be used, so just the soroban-dev and just to a soroban release version. All the workflows build rpc from soroban-tools and copy the bin to the final quickstart image, but it's only started as a service if --enable-soroban-rpc is passed on docker run and that is only supposed to be used on the soroban-dev image and the option is only allowed with --standalone and --futurenet.

Am assuming when testnet/pubnet get upgrade to soroban enabled core version, then the latest/testing workflows get changed to use same core release/ref version and would bump the soroban_tools_ref to latest version and change quickstart to allow --enable-soroban-rpc with --testnet and --pubnet and change the README.md to accurately state the same allowed usages.

for situations that want to build quickstart and use main or other ref on soroban tools, can do that locally, by checking out quickstart and running make with CORE_REF/SOROBAN_TOOLS_REF/GO_REF

2opremio · 2023-02-24T00:03:21Z

All the workflows build rpc from soroban-tools and copy the bin to the final quickstart image, but it's only started as a service if --enable-soroban-rpc is passed on docker run and that is only supposed to be used on the soroban-dev image and the option is only allowed with --standalone and --futurenet.

Does this mean that the current change will only impact the soroban-dev image?

I guess that we will need to make a new soroban-tools release so that we can bump the soroban_tools_ref to something higher that 0.6.1?

Should I just create a new tag, say, 0.7.0? (I see there's already a 0.6.1 tag without a matching release).

sreuland · 2023-02-24T00:38:23Z

All the workflows build rpc from soroban-tools and copy the bin to the final quickstart image, but it's only started as a service if --enable-soroban-rpc is passed on docker run and that is only supposed to be used on the soroban-dev image and the option is only allowed with --standalone and --futurenet.

Does this mean that the current change will only impact the soroban-dev image?

I guess that we will need to make a new soroban-tools release so that we can bump the soroban_tools_ref to something higher that 0.6.1?

Should I just create a new tag, say, 0.7.0? (I see there's already a 0.6.1 tag without a matching release).

the '0.6.1' tag on tools was done as part of Preview 7 release, just to capture a needed fix done in ingest package for rpc to use during ingestion, a gh release would have initiated other workflows that weren't desired.

Yes, the changes here are effective in the soroban-dev image build only. The soroban-rpc bin and supervisord service config files are compiled into all quickstart image variants, but it is only deployed and started when the --enable-soroban-rpc is used which also requires --standalone or --futurenet network only, and will only work on the soroban-dev image b/c it uses the latest released soroban enabled core git ref compiled with 'enable next protocol version'. As testnet and pubnet get upgraded, then at same time, will have to change quickstart to deploy and enable soroban-rpc for that network type also(follow the same as done in futurenet/soroban-rpc).

The intent of soroban-dev image was to be pinned to soroban release, we probably don't want to change soroban_tools_ref until next. There is option to build quickstart image locally with the makefile and specify newer version of tools/core refs. Once pubnet is upgraded to soroban, assuming we likely deprecate 'soroban-dev' and then just have the latest and testing images set to the latest tools/core refs.

start

2opremio · 2023-02-24T11:12:14Z

@sreuland this cannot be merged without updating thesoroban-tools ref so we have two options:

Create a v0.6.2 tag on soroban-tools repo and use that.
Put this on hold until the next soroban release

I actually think (1) is fine since the changes are only specific to soroban-rpc (although the API is changed) and don't have anything to do with the network: stellar/stellar-cli@v0.6.1...main

2opremio · 2023-02-24T11:20:57Z

I took the liberty of creating a v0.6.2 tag on soroban-tools and bumping the reference here.

2opremio · 2023-02-24T12:10:24Z

It seems like the arm64 build is getting killed (OOM?) when compiling the Rust libpreflight library:

 #12 255.7    Compiling quote v1.0.23
#12 609.6 Killed
#12 609.8 make: *** [Makefile:43: build-libpreflight] Error 137

See https://github.com/stellar/quickstart/actions/runs/4261942106/jobs/7417223417

sreuland · 2023-02-24T17:32:18Z

@sreuland this cannot be merged without updating thesoroban-tools ref so we have two options:

Create a v0.6.2 tag on soroban-tools repo and use that.

Put this on hold until the next soroban release

I actually think (1) is fine since the changes are only specific to soroban-rpc (although the API is changed) and don't have anything to do with the network: stellar/[email protected]

yes, agreed on first option and can approve if API change coming in 0.6.2 is just additive, not breaking compatibility from 0.6.1 correct?
[update] - tried out the versions on system-test, confirm that [email protected] is not compatible with [email protected] due to the removal of getAccount jrpc method, if we proceed with update, it would require using soroban-cli built with >= [email protected], or we hold here and for use cases that need latest can build quickstart locally with newer versions via the make arguments CORE_REF/SOROBAN_TOOLS_REF/GO_REF.

The Preview 7 Release also provides a pinned hash of stellar/quickstart:soroban-dev from time of release, so, for those wanting to stay on that release, they can do so.

It seems like the arm64 build is getting killed (OOM?) when compiling the Rust libpreflight library:

ok, looking at test failure, maybe a docker qemu thing..

2opremio · 2023-02-27T13:43:14Z

, if we proceed with update, it would require using soroban-cli built with >= [email protected], or we hold here and for use cases that need latest can build quickstart locally with newer versions via the make arguments CORE_REF/SOROBAN_TOOLS_REF/GO_REF.

I'm fine with one option or the other. Your call

ok, looking at test failure, maybe a docker qemu thing..

Did you get somewhere?

sreuland · 2023-02-27T17:07:34Z

, if we proceed with update, it would require using soroban-cli built with >= [email protected], or we hold here and for use cases that need latest can build quickstart locally with newer versions via the make arguments CORE_REF/SOROBAN_TOOLS_REF/GO_REF.

I'm fine with one option or the other. Your call

ok, looking at test failure, maybe a docker qemu thing..

Did you get somewhere?

@tsachiherman , do you have preference on inter-release updates to stellar/quickstart:soroban-dev, we want to confirm what the meaning of this image should represent, i.e. last stable release or 'latest', this pr introduces a newer rpc with breaking change that requires users to use the latest soroban-tools build of cli with it(the getAccount method is removed), the preview 7 release does specify a specific hash of stellar/quickstart:soroban-dev.

@2opremio , on the test failure, it does look like qemu triggering more memory, I asked about options for using gh runner with larger ram profile, the default only provides 7g, will try that out.

tsachiherman · 2023-02-27T17:13:23Z

, if we proceed with update, it would require using soroban-cli built with >= [email protected], or we hold here and for use cases that need latest can build quickstart locally with newer versions via the make arguments CORE_REF/SOROBAN_TOOLS_REF/GO_REF.

I'm fine with one option or the other. Your call

ok, looking at test failure, maybe a docker qemu thing..

Did you get somewhere?

@tsachiherman , do you have preference on inter-release updates to stellar/quickstart:soroban-dev, we want to confirm what the meaning of this image should represent, i.e. last stable release or 'latest', this pr introduces a newer rpc with breaking change that requires users to use the latest soroban-tools build of cli with it(the getAccount method is removed), the preview 7 release does specify a specific hash of stellar/quickstart:soroban-dev.

I think that you already answered your own question. Yes, we need to be able to define clearly what each one of these means, so that there is a clear definition. I think that the release docs should specify that hash, so that you could always use historical builds. That would allow us to use the latest without the hash, right ?

sreuland · 2023-02-27T19:28:54Z

I think that you already answered your own question. Yes, we need to be able to define clearly what each one of these means, so that there is a clear definition. I think that the release docs should specify that hash, so that you could always use historical builds. That would allow us to use the latest without the hash, right ?

Yes, that does imply to maintain that consistency, refs to soroban-tools from here should keep up with the changes occurring in soroban-tools inter-release so that latest from there works with latest ref used here, doesn't have to be tags on soroban-tools, can just be commit refs.

…d, preflight processing was running out of memory on 'ubuntu-latest'

tsachiherman · 2023-03-16T16:26:49Z

given that we need to start wrapping up our commitments toward the upcoming release, I'd suggest merging this in, so that we would have less technical debt toward the completing of the release. Otherwise, we need to find some way to ensure that this would get merged before the release takes place.

…an-tools and go repos

…horizon-dependency

sreuland · 2023-03-16T16:46:14Z

given that we need to start wrapping up our commitments toward the upcoming release, I'd suggest merging this in, so that we would have less technical debt toward the completing of the release. Otherwise, we need to find some way to ensure that this would get merged before the release takes place.

Ok, we had a discussion on here about what version of soroban stack that quickstart's 'soroban-dev' image should reference, agreed to make it latest, so,I've updated the pr to change 'soroban-dev' references to the mainline dev branches soroban-tools:main and go:soroban-xdr-next - 2d590c1

2opremio · 2023-03-17T10:02:14Z

Great, should we merge @sreuland ?

sreuland

yes, lgtm!

leighmcculloch · 2023-04-14T00:06:00Z

.github/workflows/build.yml

-    runs-on: ubuntu-latest
+    runs-on: ${{ inputs.core_build_runner_type }}


@2opremio Why did we switch to using a 16x cpu runner for soroban-rpc?

#431 (comment)

soroban-rpc: Remove dependency on Horizon

16fb04a

2opremio requested review from tamirms and sreuland February 23, 2023 15:43

2opremio commented Feb 23, 2023

View reviewed changes

common/soroban-rpc/bin/start Show resolved Hide resolved

Do not wait on Horizon status to show soroban-rpc's status

425325e

sreuland reviewed Feb 24, 2023

View reviewed changes

start Outdated Show resolved Hide resolved

Address review feedback

24ebb9c

Bump soroban-dev reference to v0.6.2

fb2eff3

#430: increased gh runner type to larger profile for soroban rpc buil…

0ca5d14

…d, preflight processing was running out of memory on 'ubuntu-latest'

2opremio mentioned this pull request Mar 10, 2023

remove horizon from soroban-dev image #430

Closed

sreuland added 2 commits March 16, 2023 09:42

use latest mainline commits for soroban-dev image references to sorob…

2d590c1

…an-tools and go repos

Merge remote-tracking branch 'origin/master' into soroban-rpc-remove-…

ba2025f

…horizon-dependency

sreuland approved these changes Mar 17, 2023

View reviewed changes

2opremio merged commit fedd819 into master Mar 20, 2023

2opremio deleted the soroban-rpc-remove-horizon-dependency branch March 20, 2023 08:54

sreuland mentioned this pull request Apr 6, 2023

set upcoming release tags for next branches on soroban-dev image #437

Merged

leighmcculloch reviewed Apr 14, 2023

View reviewed changes

leighmcculloch mentioned this pull request Apr 14, 2023

Change soroban-rpc to use regular runners for ci #440

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

soroban-rpc: Remove dependency on Horizon #431

soroban-rpc: Remove dependency on Horizon #431

2opremio commented Feb 23, 2023

2opremio commented Feb 23, 2023 •

edited

Loading

sreuland commented Feb 23, 2023

2opremio commented Feb 24, 2023

sreuland commented Feb 24, 2023

2opremio commented Feb 24, 2023 •

edited

Loading

2opremio commented Feb 24, 2023

2opremio commented Feb 24, 2023 •

edited

Loading

sreuland commented Feb 24, 2023 •

edited

Loading

2opremio commented Feb 27, 2023

sreuland commented Feb 27, 2023

tsachiherman commented Feb 27, 2023

sreuland commented Feb 27, 2023

tsachiherman commented Mar 16, 2023

sreuland commented Mar 16, 2023

2opremio commented Mar 17, 2023

sreuland left a comment

leighmcculloch Apr 14, 2023

2opremio Apr 14, 2023

leighmcculloch Apr 15, 2023

		runs-on: ubuntu-latest
		runs-on: ${{ inputs.core_build_runner_type }}

soroban-rpc: Remove dependency on Horizon #431

soroban-rpc: Remove dependency on Horizon #431

Conversation

2opremio commented Feb 23, 2023

2opremio commented Feb 23, 2023 • edited Loading

sreuland commented Feb 23, 2023

2opremio commented Feb 24, 2023

sreuland commented Feb 24, 2023

2opremio commented Feb 24, 2023 • edited Loading

2opremio commented Feb 24, 2023

2opremio commented Feb 24, 2023 • edited Loading

sreuland commented Feb 24, 2023 • edited Loading

2opremio commented Feb 27, 2023

sreuland commented Feb 27, 2023

tsachiherman commented Feb 27, 2023

sreuland commented Feb 27, 2023

tsachiherman commented Mar 16, 2023

sreuland commented Mar 16, 2023

2opremio commented Mar 17, 2023

sreuland left a comment

Choose a reason for hiding this comment

leighmcculloch Apr 14, 2023

Choose a reason for hiding this comment

2opremio Apr 14, 2023

Choose a reason for hiding this comment

leighmcculloch Apr 15, 2023

Choose a reason for hiding this comment

2opremio commented Feb 23, 2023 •

edited

Loading

2opremio commented Feb 24, 2023 •

edited

Loading

2opremio commented Feb 24, 2023 •

edited

Loading

sreuland commented Feb 24, 2023 •

edited

Loading