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

[poc] DO NOT MERGE: shared account ID is off curve #4371

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

s8sato
Copy link
Contributor

@s8sato s8sato commented Mar 18, 2024

Description

  • Proof of concept for the new account, with non-essential parts omitted
    • There is room for optimization in actual implementation
  • This is in the form of a draft PR just for review and not expected to be merged

Linked issue

Benefits

Checklist

  • I've read CONTRIBUTING.md
  • I've used the standard signed-off commit format (or will squash just before merging)
  • All applicable CI checks pass (or I promised to make them pass later)
  • (optional) I've written unit tests for the code changes
  • I replied to all comments after code review, marking all implemented changes with thumbs up

Signed-off-by: Shunkichi Sato <49983831+s8sato@users.noreply.github.com>
@github-actions github-actions bot added the iroha2-dev The re-implementation of a BFT hyperledger in RUST label Mar 18, 2024
use super::{PublicKey, Signatories};

/// Opaque bytes interpreted to either personal or shared account ID.
type RawId = [u8; 32];
Copy link
Contributor

Choose a reason for hiding this comment

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

haha, really suspicious

Copy link
Contributor

Choose a reason for hiding this comment

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

can we, please, not include shared accounts in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not a PR, but how multi-signatory account holds its ID will soon become important.
Since this is still intended to be a compliant proposal to the discussion so far, could you be more specific about the issue or provide an alternative?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess you just advised not to preemptively implement shared accounts in the 1st PR. If so, we are on the same page

Copy link
Contributor

Choose a reason for hiding this comment

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

If so, this PoC should be entirely re-written? I see the code is mostly about shared accounts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this PoC is a part of the specification or discussion, and is completely different from the 1st PR.
So this is expected to be closed at some point

@nxsaken nxsaken changed the base branch from iroha2-dev to main April 16, 2024 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants