Skip to content
This repository has been archived by the owner on Dec 16, 2021. It is now read-only.

fix: wallet access on seed node in local test network #466

Open
wants to merge 1 commit into
base: v0.22-dev
Choose a base branch
from

Conversation

Arne96R
Copy link

@Arne96R Arne96R commented Nov 17, 2021

Wallet access on seed node (via rpcs) in local network now works. Changed the docker-compose file so it does not feed the -masternodeblsprivkey flag to dashd when a node is not a masternode. As a result wallet functionality is not disabled in seed node, which it previously was.

Issue being fixed or feature implemented

Fixes issue https://github.com/dashevo/dashmate/issues/465.

What was done?

Change docker-compose command for core containers. Use bash to first check whether node is a masternode, if not a masternode then do not add the -masternodeblsprivkeyflag to the dashd command.

How Has This Been Tested?

Used dashmate setup local, to set up local network. Then use RPC to check whether wallet functionality on seed node works, which it does. Starting and stopping of nodes works fine.

No known effects on other code.

Breaking Changes

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Copy link
Member

@shumkov shumkov left a comment

Choose a reason for hiding this comment

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

Best practise is to create an entrypoint with condition logic instead of putting bash scripts right to the cmd. I guess we need to start with dash core entrypoint and read environment variable and add the argument mn key argument when it's present.

@Arne96R
Copy link
Author

Arne96R commented Dec 15, 2021

agreed @shumkov , but that would require changing dashcore entrypoint.sh which I wanted to avoid since I thought this is merely a dashmate issue. But I do agree that would be more neat.

Shall I then add an environment section to this docker-compose file, and add a PR on entrypoint.sh to include the logic for checking the existence of that env var? can do that, have it ready.

also, docker hub says this repo is the official repo, but in practice this image is used right? at least by dashmate.

@shumkov
Copy link
Member

shumkov commented Dec 15, 2021

@Arne96R

Shall I then add an environment section to this docker-compose file, and add a PR on entrypoint.sh to include the logic for checking the existence of that env var? can do that, have it ready.

Yeah, I believe it's the right way.

also, docker hub says this repo is the official repo, but in practice this image is used right? at least by dashmate.

For me, it's confusing too. @strophy could you please explain where the docker image is building?

One more thing, this repo is deprecated and moved to multi-package repository https://github.com/dashevo/platform/tree/v0.22-dev/packages/dashmate, so please recreate this PR there.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants