refactor: deriveAddressSeed and deriveAddress utilities
#1265
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
vadorovsky
force-pushed
the
derive-address
branch
from
a8ad93a to
e9befae
Compare
SwenSchaeferjohann
approved these changes
Sep 27, 2024
sdk/src/address.rs
Outdated
| @@ -66,19 +66,12 @@ pub fn pack_new_address_params( | |||
| /// &address_merkle_context, | |||
There was a problem hiding this comment.
Comment is not adapted
There was a problem hiding this comment.
Good catch, done
vadorovsky
changed the title
derive_address_seedderiveAddressSeed and deriveAddress utilities
vadorovsky
force-pushed
the
derive-address
branch
3 times, most recently
from
74b20a2 to
ca4a6aa
Compare
vadorovsky
force-pushed
the
derive-address
branch
5 times, most recently
from
1d4c8b8 to
862b455
Compare
me-imfhd
reviewed
Sep 30, 2024
js/stateless.js/src/utils/address.ts
Outdated
| @@ -12,13 +21,13 @@ import { getIndexOrAdd } from '../instruction'; | |||
| * defaultTestStateTreeAccounts().merkleTree | |||
| * @returns Derived address | |||
| */ | |||
| export async function deriveAddress( | |||
| export function deriveAddress( | |||
| seed: Uint8Array, | |||
| merkleTreePubkey: PublicKey = defaultTestStateTreeAccounts().merkleTree, | |||
There was a problem hiding this comment.
question: shouldn't the default be addressTree ('amt...') instead of merkleTree sorry
There was a problem hiding this comment.
Yes, it should, good catch.
There was a problem hiding this comment.
Nice, btw the comment is not consistent in the next commit
There was a problem hiding this comment.
Right. I also updated the variable name now to make it crystal clear it's address Merkle tree.
vadorovsky
force-pushed
the
derive-address
branch
4 times, most recently
from
c16ca85 to
078508a
Compare
**Problems**
- The distinction between `derive_address_seed` and `derive_address` was
unclear and we were inconsistent in it:
- We ended up applying address Merkle tree public key in both
functions, which is confusing.
- Before this change, there was no TypeScript function for deriving
**address seed**. There was only `deriveAddress`, but deriving the
unified seed was a mystery for developers.
- We have two utilities for hashing and truncating to BN254:
- `hash_to_bn254_field_size_be` - the older one, which:
- Searches for a bump in a loop, adds it to the hash inputs and then
truncates the hash. That doesn't make sense, because truncating
the hash should be sufficient, adding a bump is unnecessary.
- Another limitation is that it takes only one sequence of bytes,
making it difficult to provide multiple inputs without
concatenating them.
- `hashv_to_bn254_field_size` - the newer one, which:
- Just truncates the hash result, without the bump mechanism.
- Takes 2D byte slice as input, making it possible to pass multiple
inputs.
**Changes**
- Don't add MT pubkey in `derive_address_seed`. It's not a correct place
for it to be applied. The distinction between `derive_address_seed`
and `derive_address` should be:
- `derive_address_seed` takes provided seeds (defined by the
developer) and hashes them together with the program ID. This
operation is done only in the third-party program.
- `derive_address` takes the address seed (result of
`address_address_seed`) and hashes it together with the address
Merkle tree public key. This is done both in the third-party program
and in light-system-program. light-system-program does that as a
check whether the correct Merkle tree is used.
- Adjust the stateless.js API:
- Provide `deriveAddressSeed` function.
- Add unit tests, make sure that `deriveAddressSeed` and
`deriveAddress` provide the same results as the equivalent functions
in Rust SDK.
vadorovsky
force-pushed
the
derive-address
branch
from
078508a to
f7fc82e
Compare
SwenSchaeferjohann
approved these changes
Oct 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problems
derive_address_seedandderive_addresswasunclear and we were inconsistent in it:
functions, which is confusing.
address seed. There was only
deriveAddress, but deriving theunified seed was a mystery for developers.
hash_to_bn254_field_size_be- the older one, which:truncates the hash. That doesn't make sense, because truncating
the hash should be sufficient, adding a bump is unnecessary.
making it difficult to provide multiple inputs without
concatenating them.
hashv_to_bn254_field_size- the newer one, which:inputs.
Changes
derive_address_seed. It's not a correct placefor it to be applied. The distinction between
derive_address_seedand
derive_addressshould be:derive_address_seedtakes provided seeds (defined by thedeveloper) and hashes them together with the program ID. This
operation is done only in the third-party program.
derive_addresstakes the address seed (result ofaddress_address_seed) and hashes it together with the addressMerkle tree public key. This is done both in the third-party program
and in light-system-program. light-system-program does that as a
check whether the correct Merkle tree is used.
deriveAddressSeedfunction.deriveAddressSeedandderiveAddressprovide the same results as the equivalent functionsin Rust SDK.