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

fix: add support for signing lazily loaded transactions in genesis #3468

Merged
merged 11 commits into from
Jan 9, 2025

Conversation

zivkovicmilos
Copy link
Member

@zivkovicmilos zivkovicmilos commented Jan 9, 2025

Description

This PR fixes an issue where genesis transactions added to genesis.json through --lazy fail, since the signatures are missing.

It also introduces support for disabling genesis sig verification altogether.
Why this was needed:

  • Portal Loop transactions are signed with a valid account number and sequence (not 0), and when they are replayed (they are shoved into a new aggregated genesis.json), their signatures are also migrated. Upon initializing the chain, this would cause the signature verification to fail (the sig verification process for genesis txs expects account number and sequence values of 0, but this is not the case)

@moul, the transaction signatures in gno.land/genesis/genesis_txs.jsonl are invalid, and will always fail when being verified

@zivkovicmilos zivkovicmilos added the 📦 ⛰️ gno.land Issues or PRs gno.land package related label Jan 9, 2025
@zivkovicmilos zivkovicmilos self-assigned this Jan 9, 2025
@Gno2D2
Copy link
Collaborator

Gno2D2 commented Jan 9, 2025

🛠 PR Checks Summary

All Automated Checks passed. ✅

Manual Checks (for Reviewers):
  • IGNORE the bot requirements for this PR (force green CI check)
  • Determine if infra needs to be updated before merging (checked by @sw360cab)
Read More

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

✅ Automated Checks (for Contributors):

No automated checks match this pull request.

☑️ 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
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
Determine if infra needs to be updated before merging

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: master
    └── 🟢 Or
        ├── 🔴 A changed file matches this pattern: Dockerfile
        ├── 🔴 A changed file matches this pattern: ^misc/deployments
        ├── 🔴 A changed file matches this pattern: ^misc/docker-
        ├── 🔴 A changed file matches this pattern: ^.github/workflows/releaser.*\.yml$
        └── 🟢 A changed file matches this pattern: ^.github/workflows/portal-loop\.yml$ (filename: .github/workflows/portal-loop.yml)

Can be checked by

  • team devops

@github-actions github-actions bot added the 📦 🌐 tendermint v2 Issues or PRs tm2 related label Jan 9, 2025
Copy link

codecov bot commented Jan 9, 2025

Codecov Report

Attention: Patch coverage is 80.89888% with 17 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
gno.land/cmd/gnoland/start.go 81.13% 6 Missing and 4 partials ⚠️
gno.land/pkg/gnoland/types.go 70.00% 4 Missing and 2 partials ⚠️
gno.land/pkg/integration/pkgloader.go 0.00% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@zivkovicmilos zivkovicmilos marked this pull request as ready for review January 9, 2025 21:28
@zivkovicmilos zivkovicmilos marked this pull request as draft January 9, 2025 21:41
@zivkovicmilos
Copy link
Member Author

Fixing txtars now 🤞

@zivkovicmilos
Copy link
Member Author

@gfanton
I've updated the txtars to not be so flaky with the new changes, where we generate valid genesis tx signatures every time, and the account sequences move

@zivkovicmilos zivkovicmilos marked this pull request as ready for review January 9, 2025 23:05
@sw360cab sw360cab self-requested a review January 9, 2025 23:07
@sw360cab sw360cab merged commit 15e5896 into master Jan 9, 2025
112 checks passed
@sw360cab sw360cab deleted the dev/zivkovicmilos/lazy-genesis-sigs branch January 9, 2025 23:13
albttx pushed a commit that referenced this pull request Jan 10, 2025
…3468)

## Description

This PR fixes an issue where genesis transactions added to
`genesis.json` through `--lazy` fail, since the signatures are missing.

It also introduces support for disabling genesis sig verification
altogether.
Why this was needed:
- Portal Loop transactions are signed with a valid account number and
sequence (not 0), and when they are replayed (they are shoved into a new
aggregated `genesis.json`), their signatures are also migrated. Upon
initializing the chain, this would cause the signature verification to
fail (the sig verification process for genesis txs expects account
number and sequence values of 0, but this is not the case)

@moul, the transaction signatures in
`gno.land/genesis/genesis_txs.jsonl` are invalid, and will always fail
when being verified

---------

Co-authored-by: Nathan Toups <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related
Projects
Development

Successfully merging this pull request may close these issues.

4 participants