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

UIP-4 Spend Backreferences #4922

Merged
merged 13 commits into from
Nov 22, 2024
Merged

UIP-4 Spend Backreferences #4922

merged 13 commits into from
Nov 22, 2024

Conversation

redshiftzero
Copy link
Member

@redshiftzero redshiftzero commented Nov 13, 2024

Describe your changes

This implements UIP-4 as described in penumbra-zone/UIPs#2

Issue ticket number and link

penumbra-zone/UIPs#2

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:

@redshiftzero redshiftzero added the consensus-breaking breaking change to execution of on-chain data label Nov 13, 2024
@conorsch
Copy link
Contributor

There's a new branch release/v0.81.x where we can stage breaking changes without landing them on main. Mind retargeting the PR at that branch?

@redshiftzero redshiftzero changed the base branch from main to release/v0.81.x November 14, 2024 19:13
@redshiftzero redshiftzero marked this pull request as ready for review November 14, 2024 23:48
@redshiftzero redshiftzero changed the title wip: UIP-4 Spend Backreferences UIP-4 Spend Backreferences Nov 14, 2024
@cronokirby cronokirby self-requested a review November 15, 2024 17:11
Copy link
Contributor

@cronokirby cronokirby left a comment

Choose a reason for hiding this comment

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

I think we should make encryption non-fallible, otherwise LGTM

crates/core/component/shielded-pool/src/spend/action.rs Outdated Show resolved Hide resolved
&self,
brk: &BackreferenceKey,
nullifier: &Nullifier,
) -> Result<Option<Backref>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think having Result is smart here, because then you can distinguish between having not put in a back reference, in which case maybe the reference would have pointed to a spend of yours, and a reference you know doesn't belong to you because you can't decrypt it.

This is safe because we know the arrays that have been allocated
are the correct size
@cronokirby cronokirby merged commit ac37c0f into release/v0.81.x Nov 22, 2024
12 checks passed
@cronokirby cronokirby deleted the uip4-impl branch November 22, 2024 16:21
conorsch pushed a commit that referenced this pull request Nov 22, 2024
## Describe your changes

This implements UIP-4 as described in
penumbra-zone/UIPs#2

## Issue ticket number and link

penumbra-zone/UIPs#2

## Checklist before requesting a review

- [ ] 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:
conorsch pushed a commit that referenced this pull request Dec 13, 2024
## Describe your changes

This implements UIP-4 as described in
penumbra-zone/UIPs#2

## Issue ticket number and link

penumbra-zone/UIPs#2

## Checklist before requesting a review

- [ ] 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:
conorsch pushed a commit that referenced this pull request Dec 14, 2024
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:
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.

4 participants