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

Da voter config for docker compose #513

Merged
merged 3 commits into from
Nov 7, 2023
Merged

Da voter config for docker compose #513

merged 3 commits into from
Nov 7, 2023

Conversation

bacv
Copy link
Member

@bacv bacv commented Nov 6, 2023

Configurable da voter and minor improvements to docker compose file.
A copy of this closed PR: #512

@bacv bacv added the testnet label Nov 6, 2023
@bacv bacv added this to the Nomos testnet (playground) milestone Nov 6, 2023
@bacv bacv self-assigned this Nov 6, 2023
Copy link
Contributor

@youngjoon-lee youngjoon-lee left a comment

Choose a reason for hiding this comment

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

👍

Comment on lines 274 to 280
pub fn update_da(mut self, da_args: DaArgs) -> Result<Self> {
let DaArgs { da_voter_key } = da_args;

if let Some(private_key) = da_voter_key {
let bytes = <[u8; 32]>::from_hex(private_key)?;
self.da.da_protocol.voter = bytes;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you. I forgot to add this function here.

I think it might be clearer to not call this private key, since we don't know yet what will be used for the voter, as discussed in #498 (comment) (even though we'll anyway use the same private key as the consensus service for now).

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, that makes sense! Updated.

Copy link
Contributor

@zeegomo zeegomo left a comment

Choose a reason for hiding this comment

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

Agree with #513 (comment), the rest is good

@bacv bacv force-pushed the docker-compose-da-voter branch 5 times, most recently from a116492 to d151529 Compare November 7, 2023 11:26
@bacv
Copy link
Member Author

bacv commented Nov 7, 2023

Just for FYI, github didn't let me merge, because for some reason commits were not signed. Even after updating the signing key, I had to sign every commit in this PR. Did it with this command:

 git rebase --exec 'git commit --amend --no-edit -n -S' -i <sign-until-this-hash>

@bacv bacv merged commit c14998b into master Nov 7, 2023
7 checks passed
@bacv bacv deleted the docker-compose-da-voter branch November 7, 2023 11:52
al8n pushed a commit that referenced this pull request Nov 8, 2023
* Readd docker build context

* Configurable da protocol voter

* Do not use private key naming for da voter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants