-
Notifications
You must be signed in to change notification settings - Fork 5
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
Update make
based instructions
#260
Conversation
- Link to native libs - Fix CLI args
- Based on `config-files/operator0-docker-compose.anvil.yaml`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would propose to refrain from getting rollup-data-availability as submodule.
We shall list rollup-data-availability artifacts as prerequisites. Maybe with explanation on how to compile it.
The compilation of relayer is platform-dependant and rollup-data-availability handles that here and here.
User shall invoke make command here and then move it to /usr/local/lib. Go will pick it up itself after that
May I suggest another angle, How about instead of using |
That is a great idea. @emlautarom1 do you want to play with it? This would require small Rust service on top of rollup-data-availability. |
We already have a service for this in the repo! https://github.com/Nuffle-Labs/data-availability/blob/main/bin/http-api/src/main.rs |
Initially I worked on a different approach that relied on cloning the I'm against installing the libraries globally (ex. I think defining the make command to something like
My understanding is that our relayer ( |
Regarding the alternative proposed by @firatsertgoz, I think it would be nice to explore it later. For now I would like to fix the current |
- Scope `CGO_LDFLAGS` to single command
About the Go sidecar conversation - I agree with both. We should move to the Go sidecar but IMO this PR should keep its scope. Reason for that is already in #165. We can move ahead with it, but I'd still prefer if it kept a compatibility layer in this case. AFAIK we don't have other breaking updates piled up and coordinating those should be very costly - there are nodes from all over the world and some are more responsive than others. |
@Hyodar thanks for implicitly reminding about the PR)
The thing is that Unfortunately there's no such thing as build.rs in GO |
Okay, all conversation aside, I think the first question we should answer here is whether this tooling is important atm. It was used before and increasingly unused over time. Are we using these individual commands, or should we just deprecate this and use exclusively docker compose containers? On my end, I usually just use the containers directly, as it's actually closer to the real scenario. |
I favor deprecating the |
Okay, after checking the whole conversation again, we can generally move forward with this. My overall take is:
That said, I understand the trouble here, but overall I think defining the library as it is the best approach for now, though I'd highly prefer if we concatted (e.g. Again, this should be deprecated really soon in favor of only container-based testing, so we don't have much to gain on this discussion. |
Done, now we concatenate existing
Already updated the main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Please update the docs/
intro page as well.
Added the deprecation of the non-container testing environment instructions as #269. |
Merging as discussed during today's daily |
Current Behavior
make start-operator
errors out immediately with the following error:More details on #257
New Behavior
This PR updates
make
based instructions to match the currentDocker
setup.Breaking Changes
None beside getting something to work that was previously broken.
Observations
near-da-rpc-sys
make command that builds the native libraries required by the relayer.CGO
linker flags so the relayer can pick the required native libraries during build time.README
with instructions on how to build the native libraries.config-files/operator.anvil.yaml
to match the latest file structure regarding keys.