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

backport v0.81.0 changes to main #4969

Merged
merged 9 commits into from
Dec 20, 2024
Merged

backport v0.81.0 changes to main #4969

merged 9 commits into from
Dec 20, 2024

Conversation

conorsch
Copy link
Contributor

Describe your changes

This PR cherry-picks the contents of the release/v0.81.x onto the main branch.

Issue ticket number and link

N/A

To review

  1. Ensure that this PR contains only commits cherry-picked from release/v0.81.x, nothing new
  2. Ensure that this PR contains all commits from release/v0.81.x, with nothing missed

We'll need to maintain the release/v0.81.x branch indefinitely, since that's where the v0.81.0 tag resides, but that's fine: we do that for all historical release branches, and branch protections are enabled on Github to prevent accidents.

Checklist before requesting a review

  • I have added guiding text to explain how a reviewer should test these changes.

  • If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason:

    I am labeling this as consensus-breaking, because it the protocol changes in release/v0.81.x constitute breakage for the 0.80.x series.

redshiftzero and others added 8 commits December 20, 2024 11:17
This implements UIP-4 as described in
penumbra-zone/UIPs#2

penumbra-zone/UIPs#2

- [ ] I have added guiding text to explain how a reviewer should test
these changes.

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:

(cherry picked from commit 067708b)
Implementation of the candidate UIP-5: outbound PFM support (see
https://forum.penumbra.zone/t/pre-uip-outbound-packet-forwarding-middleware-support/121)

Should be reviewed for correctness and adherence to the spec.

Note that this only includes the required protocol changes to allow
clients to use the new memo field (a prerequisite for PFM support), and
does not implement outbound packet forwarding directly (that is to be
done in the client).

By @avahowell cherry-picked from #4923

Co-authored-by: Ava Howell <[email protected]>
(cherry picked from commit 5a66403)
This implements a migration to version 0.81.0, which doesn't require any
logic.

This will break PD, but targets the new release branch. The APP_VERSION has been incremented to 9, in order to satisfy the safeguard version checks from UIP-6.

---------

Co-authored-by: Conor Schaefer <[email protected]>
(cherry picked from commit 7e0825b)
This commit experiments with a "truncated" address format, which aims at
maximizing compatibility with other chains. This is kind of a janky
hack, but may be worth doing anyways -- and could allow users to work
around current and future Bech32m encoding compatibility issues (though
our goal should be to ensure compatibility).

- Uses Bech32, like other Cosmos chains
- Fits in 32 bytes -- looks exactly like a cosmos except for a different
Bech32 HRP
- As an alternate encoding, requires no changes to other parts of
Penumbra, can be parsed directly into an `Address`

- A third address format adds UX complexity
- The truncated address points at a random account number
- Only one truncated address per IVK (reuse of the address causes
linkability)
- Not compatible with FMD (clues created with the truncated address are
not detectable)

A Penumbra address has three components: the diversifier `d`, the
transmission key `pk_d`, and the clue key `ck_d`. To truncate the
address to a 32-byte encoding:
- The diversifier is required to be `[0; 16]`, omitted from the
encoding, and filled in with the hardcoded value during decoding;
- The transmission key is encoded as 32 bytes;
- The clue key is stripped, and filled in on the decoding side with the
identity element.

The fixed diversifier means that there's only one per IVK, and also that
the truncated address points at a random sub-account: the fixed value
`[0u8; 16]` is decrypted with the diversifier key to a random address
index. Stripping the clue key and filling it in with a dummy value means
that generated clues are not detectable, but should not have other
effects on the protocol, and since FMD is currently unused this seems
fine.

To allow the use of truncated addresses in IBC transfers, the
`Ics20Withdrawal` action would need to be changed to have a
`use_truncated_address` parameter.

No issue - starting point for forum discussion

- [ ] ~~I have added guiding text to explain how a reviewer should test
these changes.~~ N/A - starting point for discussion

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label.

---------

Co-authored-by: redshiftzero <[email protected]>
(cherry picked from commit ff94498)
## Describe your changes

This PR makes sure that truncated addresses are encoded correctly when
ICS20 withdrawals are marshaled into fungible packet data.

In addition to that, it modifies pcli to (hazardously?) handcraft a
return `Address` with an identity clue key and zero diversifier.

To test this PR, one can performa IBC roundtrip against a testnet chain
with broken bech32m handling (e.g, `grand-1`).

## Issue ticket number and link

#4950

## Checklist before requesting a review

- [x] I have added guiding text to explain how a reviewer should test
these changes.

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:

  > CB release branch.

(cherry picked from commit c4ab7d7)
## Describe your changes

This PR:
- logs state divergences when processing `EndBlock` messages
- prevents `pd` operators from migrating corrupted state unless
`--force` is on

## Checklist before requesting a review

- [x] I have added guiding text to explain how a reviewer should test
these changes: simple control flow.

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:

(cherry picked from commit c9b5db8)
Co-authored-by: Lucas Meier <[email protected]>
(cherry picked from commit c08b2ef)
(cherry picked from commit 8df36a4)
@conorsch conorsch added the consensus-breaking breaking change to execution of on-chain data label Dec 20, 2024
@conorsch conorsch requested a review from erwanor December 20, 2024 19:21
@erwanor erwanor merged commit 2248e21 into main Dec 20, 2024
14 checks passed
@erwanor erwanor deleted the port-0_81_0-changes-to-main branch December 20, 2024 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus-breaking breaking change to execution of on-chain data
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants