Implemented Merkle Airdrop Support for Initial Token Distribution #73
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@abdelhamidbakhta I have low trust in alexandria libs - can we get someone to review them? |
|
Resolves #28 @Akashneelesh (dont forget) to link issues to PRs |
|
Sorry, I added a lot of minor changes so that the tests are clean and standardized. The biggest issue might be in the claim airdrop function. I'm not sure how the amount is defined or who will call this function |
Firstly to answer about who will call this : The users would call this function. Amount is defined in the list of addressAmountPairs (you could take a look at the scripts/merkletree.ts). So I did take this into consideration, and because the leaf (pedersen hash of the to_address and amount) , The Merkle proof generated for a particular leaf in a Merkle tree changes based on the values in the tree. And hence in the contract, theres an assertion of the hashed_value(to_address,amount) and the leaf. Only if that goes through then the proof is verified. And the amount is minted. |
| //Pedersen Hashing of the ContractAddress and Amount | ||
| let to_felt252: felt252 = starknet::contract_address_to_felt252(to); | ||
| let amount_felt252: felt252 = amount.try_into().unwrap(); | ||
| let hashed_value: felt252 = pedersen::pedersen(to_felt252, amount_felt252); |
There was a problem hiding this comment.
contractAddress fits into felt252, but not amount. Here you're performing a hash, not on the original data, but on a truncating value. why not hashing the real amount (on 256 bits)?
There was a problem hiding this comment.
Yeah true, I had casted amount to felt252 because, the pedersen::pedersend requires (felt252,felt252) as inputs.
There was a problem hiding this comment.
I think we should hash the full amount 🤔 I will look into alexandria's merkle tree implementation but I think it can be improved 🤔
There was a problem hiding this comment.
imo you should
let mut hash_state = pedersen(to_felt252, amount.low.into())
hash_state = pedersen(hash_state, amount.high.into())
that way you are hashing the correct data
There was a problem hiding this comment.
Ohh fine, I'll do this change
@enitrat what is the issue with the alexandria libs? Also I am not sure to understand why we are using pedersen when poseidon is cheaper? EDIT: Is it because starknet.js manage only pedersen hash in its merkle feature? import { merkle, ec } from "starknet"; |
there's pretty much no maintenence of already existing stuff, with the language rapidly evolving I would think some patterns are un-optimized / outdated and some design decisions a bit weird (e.g the way hash function are used in the merkle trees are implemented is super weird) |
|
|
||
| fn get_merkle_root(self: @ContractState) -> felt252 { | ||
| //Getting the merkle root | ||
| self.ownable.assert_only_owner(); |
There was a problem hiding this comment.
Why is there an access control on this view?
I have taken all the description and the changes mentioned in the telegram group into consideration and have done the changes.