-
Notifications
You must be signed in to change notification settings - Fork 19
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
Generate a random Drip List ID when creating a new Drip List #1406
Changes from 5 commits
29f086c
0f3ed61
ae7e5dd
1a7ea18
7c1c98f
be67046
47fb8ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
import calculateSaltFromAddress from '../calc-salt'; | ||
|
||
describe('calculate salt', () => { | ||
it('calculates a random salt', () => { | ||
const SEED_CONSTANT = 'test'; | ||
const address = '0xe2E9b9B5d0757c26aB477A754788B19b60f2ed83'; | ||
|
||
const results = [ | ||
calculateSaltFromAddress(SEED_CONSTANT, address), | ||
calculateSaltFromAddress(SEED_CONSTANT, address), | ||
calculateSaltFromAddress(SEED_CONSTANT, address), | ||
calculateSaltFromAddress(SEED_CONSTANT, address), | ||
calculateSaltFromAddress(SEED_CONSTANT, address), | ||
calculateSaltFromAddress(SEED_CONSTANT, address), | ||
calculateSaltFromAddress(SEED_CONSTANT, address), | ||
calculateSaltFromAddress(SEED_CONSTANT, address), | ||
calculateSaltFromAddress(SEED_CONSTANT, address), | ||
calculateSaltFromAddress(SEED_CONSTANT, address), | ||
]; | ||
|
||
const set = new Set(results); | ||
expect(set.size).toBe(results.length); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import { ethers } from 'ethers'; | ||
|
||
export default function calculateSaltFromAddress(seedConstant: string, address: string) { | ||
const hash = ethers.keccak256( | ||
ethers.AbiCoder.defaultAbiCoder().encode(['string'], [seedConstant + address]), | ||
); | ||
const addressBigInt = ethers.toBigInt('0x' + hash.slice(26)); | ||
|
||
const randomBytes = ethers.randomBytes(32); | ||
const randomBigInt = BigInt(ethers.hexlify(randomBytes)); | ||
|
||
return addressBigInt & randomBigInt & BigInt('0xFFFFFFFFFFFFFFFF'); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
import NftDriverMetadataManager from '../metadata/NftDriverMetadataManager'; | ||
import MetadataManagerBase from '../metadata/MetadataManagerBase'; | ||
import { ethers, MaxUint256, type Signer, toBigInt } from 'ethers'; | ||
import { MaxUint256, type Signer, toBigInt } from 'ethers'; | ||
import GitProjectService from '../project/GitProjectService'; | ||
import assert from '$lib/utils/assert'; | ||
import type { Address, IpfsHash } from '../common-types'; | ||
|
@@ -9,12 +9,6 @@ import { get } from 'svelte/store'; | |
import Emoji from '$lib/components/emoji/emoji.svelte'; | ||
import type { nftDriverAccountMetadataParser } from '../metadata/schemas'; | ||
import type { LatestVersion } from '@efstajas/versioned-parser'; | ||
import { gql } from 'graphql-request'; | ||
import query from '$lib/graphql/dripsQL'; | ||
import type { | ||
MintedNftAccountsCountQuery, | ||
MintedNftAccountsCountQueryVariables, | ||
} from './__generated__/gql.generated'; | ||
import type { Items, Weights } from '$lib/components/list-editor/types'; | ||
import { buildStreamCreateBatchTx } from '../streams/streams'; | ||
import { | ||
|
@@ -34,6 +28,7 @@ import keyValueToMetatada from '../sdk/utils/key-value-to-metadata'; | |
import { populateCallerWriteTx } from '../sdk/caller/caller'; | ||
import txToCallerCall from '../sdk/utils/tx-to-caller-call'; | ||
import network from '$lib/stores/wallet/network'; | ||
import calculateSaltFromAddress from '../calc-salt'; | ||
|
||
type AccountId = string; | ||
|
||
|
@@ -113,23 +108,7 @@ export default class DripListService { | |
items, | ||
); | ||
|
||
const mintedNftAccountsCountQuery = gql` | ||
query MintedNftAccountsCount($ownerAddress: String!, $chain: SupportedChain!) { | ||
mintedTokensCountByOwnerAddress(ownerAddress: $ownerAddress, chain: $chain) { | ||
total | ||
} | ||
} | ||
`; | ||
|
||
const mintedNftAccountsCountRes = await query< | ||
MintedNftAccountsCountQuery, | ||
MintedNftAccountsCountQueryVariables | ||
>(mintedNftAccountsCountQuery, { ownerAddress: this._ownerAddress, chain: network.gqlName }); | ||
|
||
const salt = this._calcSaltFromAddress( | ||
this._ownerAddress, | ||
mintedNftAccountsCountRes.mintedTokensCountByOwnerAddress.total ?? 0, | ||
); | ||
const salt = this._calcSaltFromAddress(this._ownerAddress); | ||
|
||
const listId = ( | ||
await executeNftDriverReadMethod({ | ||
|
@@ -368,25 +347,8 @@ export default class DripListService { | |
return ipfsHash; | ||
} | ||
|
||
// We use the count of *all* NFT sub-accounts to generate the salt for the Drip List ID. | ||
// This is because we want to avoid making HTTP requests to the subgraph for each NFT sub-account to check if it's a Drip List. | ||
private _calcSaltFromAddress = (address: string, listCount: number): bigint /* 64bit */ => { | ||
const hash = ethers.keccak256( | ||
ethers.AbiCoder.defaultAbiCoder().encode(['string'], [this.SEED_CONSTANT + address]), | ||
); | ||
const randomBigInt = ethers.toBigInt('0x' + hash.slice(26)); | ||
|
||
let random64BitBigInt = BigInt(randomBigInt.toString()) & BigInt('0xFFFFFFFFFFFFFFFF'); | ||
|
||
const listCountBigInt = BigInt(listCount); | ||
random64BitBigInt = random64BitBigInt ^ listCountBigInt; | ||
|
||
return random64BitBigInt; | ||
}; | ||
|
||
private _generateDripIdFromSalt = (salt: bigint): number /* 32bit */ => { | ||
const random32BitNumber = Number(salt & BigInt(0xffffffff)); | ||
|
||
return random32BitNumber; | ||
// Create random salt from address | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we sure this generates a random number? (Even the previous code, I believe wasn't truly random.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're not sure and it looks like it doesn't based on the error I'm receiving. Will look into it! |
||
private _calcSaltFromAddress = (address: string): bigint /* 64bit */ => { | ||
return calculateSaltFromAddress(this.SEED_CONSTANT, address); | ||
}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This definitely introduces randomness now 👍
But, why not simplifying to something simple like:
This is purely random compared to what we had before where the final result is partially influenced by the (deterministic)
seed
andaddress
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No reason at all. Just not trying to shake the tree too much because I don't know what constraints the seed should have. Will update!