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

Remove MASP pin key #2675

Closed
Tracked by #2530 ...
grarco opened this issue Feb 20, 2024 · 4 comments · Fixed by #3142
Closed
Tracked by #2530 ...

Remove MASP pin key #2675

grarco opened this issue Feb 20, 2024 · 4 comments · Fixed by #3142
Assignees
Labels
breaking:consensus Consensus breaking change that requires a hard-fork MASP pre-mainnet Must happen before mainnet.

Comments

@grarco
Copy link
Collaborator

grarco commented Feb 20, 2024

Currently, the masp optional pin key is used in an inconsistent way: when a user generates a payment address there’s the chance to pin this address, if this is the case than all transactions with this payment address as the receiver will produce a pin key with the hash of the address, otherwise no pin key will be produced. If no pin is requested when generating the address, then no pin key will be produced.

Also, since we produce the pin key in a deterministic way (hash of the payment address) I believe we end up rewriting the same key in storage.

@grarco grarco added the MASP label Feb 20, 2024
@grarco grarco added the pre-mainnet Must happen before mainnet. label Mar 13, 2024
@grarco
Copy link
Collaborator Author

grarco commented Apr 11, 2024

In the end, after some talk we decided to remove the pin key and focus on developing a better implementation of the shielded sync process #2900.

cc: @Fraccaman, @batconjurer

@cwgoes
Copy link
Collaborator

cwgoes commented Apr 20, 2024

In that case I'm going to close this issue.

@cwgoes cwgoes closed this as completed Apr 20, 2024
@grarco grarco reopened this Apr 22, 2024
@grarco
Copy link
Collaborator Author

grarco commented Apr 22, 2024

Reopening as this has not been completed yet

@cwgoes cwgoes changed the title MASP pin key Remove MASP pin key Apr 22, 2024
@cwgoes
Copy link
Collaborator

cwgoes commented Apr 22, 2024

I see, sorry, it wasn't clear to me what task needed to be completed. Updated the title to clarify.

@grarco grarco self-assigned this Apr 26, 2024
grarco added a commit that referenced this issue Apr 26, 2024
@grarco grarco mentioned this issue Apr 26, 2024
2 tasks
@grarco grarco added the breaking:consensus Consensus breaking change that requires a hard-fork label Apr 26, 2024
grarco added a commit that referenced this issue Apr 27, 2024
brentstone added a commit that referenced this issue Apr 30, 2024
* grarco/remove-pin-key:
  Removes `query_tx_deltas`
  Changelog #2675
  Removes unused errors and integration test
  Fixes econding. Updates shielded keys and addresses for tests
  Adjusts wasm txs and client args
  Removes masp pin key
  Rmoves old allowlisted gas
@grarco grarco mentioned this issue May 3, 2024
2 tasks
grarco added a commit that referenced this issue May 3, 2024
brentstone added a commit that referenced this issue May 7, 2024
* grarco/remove-delta-map:
  Changelog #3172
  Removes `delta_map` from the `ShieldedContext` and all the code to work with the balance keys
  Removes `query_tx_deltas`
  Changelog #2675
  Removes unused errors and integration test
  Fixes econding. Updates shielded keys and addresses for tests
  Adjusts wasm txs and client args
  Removes masp pin key
  Rmoves old allowlisted gas
brentstone added a commit that referenced this issue May 8, 2024
* origin/grarco/remove-pin-key:
  Removes `query_tx_deltas`
  Changelog #2675
  Removes unused errors and integration test
  Fixes econding. Updates shielded keys and addresses for tests
  Adjusts wasm txs and client args
  Removes masp pin key
  Rmoves old allowlisted gas
brentstone added a commit that referenced this issue May 8, 2024
* origin/grarco/remove-delta-map:
  Changelog #3172
  Removes `delta_map` from the `ShieldedContext` and all the code to work with the balance keys
  Removes `query_tx_deltas`
  Changelog #2675
  Removes unused errors and integration test
  Fixes econding. Updates shielded keys and addresses for tests
  Adjusts wasm txs and client args
  Removes masp pin key
  Rmoves old allowlisted gas
brentstone added a commit that referenced this issue May 9, 2024
* origin/grarco/remove-pin-key:
  Removes `query_tx_deltas`
  Changelog #2675
  Removes unused errors and integration test
  Fixes econding. Updates shielded keys and addresses for tests
  Adjusts wasm txs and client args
  Removes masp pin key
  Rmoves old allowlisted gas
brentstone added a commit that referenced this issue May 9, 2024
* origin/grarco/remove-delta-map:
  Changelog #3172
  Removes `delta_map` from the `ShieldedContext` and all the code to work with the balance keys
  Removes `query_tx_deltas`
  Changelog #2675
  Removes unused errors and integration test
  Fixes econding. Updates shielded keys and addresses for tests
  Adjusts wasm txs and client args
  Removes masp pin key
  Rmoves old allowlisted gas
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking:consensus Consensus breaking change that requires a hard-fork MASP pre-mainnet Must happen before mainnet.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants