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.

Sign up for GitHub

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

Jump to bottom

Update PoP generation spec #432

Merged
merged 3 commits into from
Jan 30, 2025
Merged

Update PoP generation spec #432

merged 3 commits into from
Jan 30, 2025

Conversation

gitferry
Copy link
Member

This PR:

  • moved pop spec to the begining of each section
  • modified PR submission guide

@gitferry gitferry requested a review from vitsalis January 30, 2025 03:14
KonradStaniec
KonradStaniec previously approved these changes Jan 30, 2025
View reviewed changes
Copy link
Contributor

@KonradStaniec KonradStaniec left a comment

Choose a reason for hiding this comment

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

One suggestion about completness of adr36 payload, non blocking if you think it is overkill

bbn-1/pops/README.md
- `EotsSignBaby`: A Schnorr signature in base64 format, created by signing the
sha256(`BabyAddress`) with the EOTS private key.
- `BabySignEotsPk`: A signature of the EotsPublicKey, created by the Babylon
private key. This signature follows the Cosmos ADR-036 specification and is
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it is worth mentioning whole json payload that needs to be signed ? i.e

{
  "chain_id": "",
  "account_number": "0",
  "sequence": "0",
  "fee": {
    "gas": "0",
    "amount": []
  },
  "msgs": [
    {
      "type": "sign/MsgSignData",
      "value": {
        "signer": "<signer_address>",
        "data": "<base64_encoded_data>"
      }
    }
  ],
  "memo": ""
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. Added the spec reference

@gitferry gitferry merged commit 7bc73a0 into main Jan 30, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KonradStaniec KonradStaniec KonradStaniec approved these changes

@RafilxTenfen RafilxTenfen RafilxTenfen approved these changes

@fishermanymc fishermanymc Awaiting requested review from fishermanymc

@filippos47 filippos47 Awaiting requested review from filippos47

@vitsalis vitsalis Awaiting requested review from vitsalis

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@gitferry @KonradStaniec @RafilxTenfen