Skip to content

chore: update multisig docs: default sorting, derivation guidance and examples#5117

Open
D4ryl00 wants to merge 4 commits intognolang:masterfrom
D4ryl00:chore/update-multisig-doc
Open

chore: update multisig docs: default sorting, derivation guidance and examples#5117
D4ryl00 wants to merge 4 commits intognolang:masterfrom
D4ryl00:chore/update-multisig-doc

Conversation

@D4ryl00
Copy link
Contributor

@D4ryl00 D4ryl00 commented Feb 3, 2026

  • Clarify that gnokey add multisig sorts member pubkeys by default (-nosort=false), so member order doesn’t matter.
  • Recommend using a dedicated key for multisig participation and add derivation examples (account/index and explicit path).

depends on #5122

Signed-off-by: D4ryl00 <d4ryl00@gmail.com>
Signed-off-by: D4ryl00 <d4ryl00@gmail.com>
@Gno2D2
Copy link
Collaborator

Gno2D2 commented Feb 3, 2026

🛠 PR Checks Summary

All Automated Checks passed. ✅

Manual Checks (for Reviewers):
  • IGNORE the bot requirements for this PR (force green CI check)
  • The pull request description provides enough details
Read More

🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.

✅ Automated Checks (for Contributors):

🟢 Maintainers must be able to edit this pull request (more info)
🟢 Pending initial approval by a review team member, or review from tech-staff

☑️ Contributor Actions:
  1. Fix any issues flagged by automated checks.
  2. Follow the Contributor Checklist to ensure your PR is ready for review.
    • Add new tests, or document why they are unnecessary.
    • Provide clear examples/screenshots, if necessary.
    • Update documentation, if required.
    • Ensure no breaking changes, or include BREAKING CHANGE notes.
    • Link related issues/PRs, where applicable.
☑️ Reviewer Actions:
  1. Complete manual checks for the PR, including the guidelines and additional checks if applicable.
📚 Resources:
Debug
Automated Checks
Maintainers must be able to edit this pull request (more info)

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 The pull request was created from a fork (head branch repo: D4ryl00/gno)

Then

🟢 Requirement satisfied
└── 🟢 Maintainer can modify this pull request

Pending initial approval by a review team member, or review from tech-staff

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 Not (🔴 Pull request author is a member of the team: tech-staff)

Then

🟢 Requirement satisfied
└── 🟢 If
    ├── 🟢 Condition
    │   └── 🟢 Or
    │       ├── 🔴 At least one of these user(s) reviewed the pull request: [jefft0 notJoon omarsy MikaelVallenet] (with state "APPROVED")
    │       ├── 🟢 At least 1 user(s) of the team tech-staff reviewed pull request
    │       └── 🔴 This pull request is a draft
    └── 🟢 Then
        └── 🟢 Not (🔴 This label is applied to pull request: review/triage-pending)

Manual Checks
**IGNORE** the bot requirements for this PR (force green CI check)

If

🟢 Condition met
└── 🟢 On every pull request

Can be checked by

  • Any user with comment edit permission
The pull request description provides enough details

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 Not (🔴 Pull request author is a member of the team: core-contributors)
    └── 🟢 Not (🔴 Pull request author is user: dependabot[bot])

Can be checked by

  • team core-contributors

Signed-off-by: D4ryl00 <d4ryl00@gmail.com>
@D4ryl00 D4ryl00 marked this pull request as ready for review February 5, 2026 10:36
@Gno2D2 Gno2D2 added the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Feb 5, 2026
@jefft0
Copy link
Contributor

jefft0 commented Feb 5, 2026

#5122 is the issue. Maybe you should say this depends on merging #5123?

@D4ryl00
Copy link
Contributor Author

D4ryl00 commented Feb 5, 2026

#5122 is the issue. Maybe you should say this depends on merging #5123?

My logic is that the PR is blocked by an issue/problem (whatever which PRs would solve it, I don't care). Until this problem exists, we can't merge this PR.

Comment on lines 850 to 852
`gnokey add multisig` sorts member pubkeys by default (`-nosort=false`), so the
creation order does **not** matter. The multisig has a deterministic member order
based on pubkey sorting, not on the CLI order you provided.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Consider consolidating the "order does not matter" reference into a single location and referencing it instead with something like as mentioned above, order is handled automatically. The issue is that having same/similar explanations in multiple places will make it harder to maintain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, I simplified. Let me know if it's not enough.
done in b9fb511

up with the same multisig public key/address, and signing will fail.

If you set `-nosort=true`, then ordering matters and everyone must use the same
explicit order.
Copy link
Member

Choose a reason for hiding this comment

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

nit: When set, go flags are implicitly true, so -nosort=true is redundant.

Suggested change
explicit order.
If you set `-nosort`, then ordering matters and everyone must use the same
explicit order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, done in b9fb511

@Gno2D2 Gno2D2 removed the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Feb 5, 2026
@jefft0 jefft0 moved this to Other in review in 🤝🏻 Partner: Berty Feb 5, 2026
Signed-off-by: D4ryl00 <d4ryl00@gmail.com>
@D4ryl00 D4ryl00 force-pushed the chore/update-multisig-doc branch from 712dc2b to b9fb511 Compare February 5, 2026 13:43
@D4ryl00 D4ryl00 requested a review from gfanton February 5, 2026 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📖 documentation Improvements or additions to documentation

Projects

Status: No status
Status: Other in review

Development

Successfully merging this pull request may close these issues.

4 participants