-
Notifications
You must be signed in to change notification settings - Fork 21
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
feat: on-chain collections whitelist #1339
base: main
Are you sure you want to change the base?
Conversation
…dexer. Add whitelisted prop to Collection and update it by listening add/remove from cw-address-list contract
…new version, remove old script, make generate
✅ Deploy Preview for testitori ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for teritori-dapp ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
None => Ok(()), | ||
})?; | ||
|
||
Ok(Response::default()) | ||
return Ok(Response::new() | ||
.add_attribute("action", "add_whitelisted_collection") |
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.
keep the contract generic, it's used for the marketplace whitelist but it's actually just a list of addresses, we could use it for many other things
also the convention is to use the method name as action I believe
.add_attribute("action", "add_whitelisted_collection") | |
.add_attribute("action", "add") |
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.
Oki
842ce2b
|
||
Ok(Response::default()) | ||
return Ok(Response::new() | ||
.add_attribute("action", "remove_whitelisted_collection") |
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.
.add_attribute("action", "remove_whitelisted_collection") | |
.add_attribute("action", "remove") |
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.
Ok(Response::default()) | ||
return Ok(Response::new() | ||
.add_attribute("action", "add_whitelisted_collection") | ||
.add_attribute("collection_addr", addr)); |
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.
.add_attribute("collection_addr", addr)); | |
.add_attribute("added_addr", addr)); |
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.
Ok(Response::default()) | ||
return Ok(Response::new() | ||
.add_attribute("action", "remove_whitelisted_collection") | ||
.add_attribute("collection_addr", addr)); |
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.
.add_attribute("collection_addr", addr)); | |
.add_attribute("removed_addr", addr)); |
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.
if execMsg.Contract != h.config.Network.VaultContractAddress { | ||
return nil | ||
} |
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 can't work, the whitelist is not the marketplace contract
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.
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.
there is still a problem, we should check that the contract is actually the whitelist contract, are you testing this?
you can't rely on events for security, anyone can create any event, you need to make sure you are in the correct context
also since this is supposed to be managed by a DAO, the handler should be matched also in the DAO proposal execution case
in all cases, your handlers must be restricted to only run when the execMsg.Contract
is the whitelist contract
you must stat to think about how your code can be exploited, in this specific case, how the whitelist could be manipulated without being the admin. it's not only in the contract but also in the backend services
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.
It's a big mistake, CwAdminFactory is off topic wtf sorry. I'll commit the right code
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.
…act, remove useless codeId from feature, enhance script, make generate
@@ -62,7 +62,7 @@ export const teritoriTestnetNetwork: CosmosNetworkInfo = { | |||
"https://explorer.teritori.com/teritori-testnet/account/$address", | |||
idPrefix: "testori", | |||
testnet: true, | |||
backendEndpoint: "https://dapp-backend.testnet.teritori.com", | |||
backendEndpoint: "http://192.168.1.78:9090", |
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.
revert this pls
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.
My apologies for that..
439ae84
✅ Deploy Preview for gno-dapp ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
we should also edit the collections queries to use this new whitelist column
The purpose is to replace this actual Collections Whitelist handling:
Actually, the whitelisted collections are in this env variable:
TERITORI_COLLECTION_WHITELIST=testeth-0x43cc70bf324d716782628bed38af97e4afe92f69,mumbai-0x916ad9d549907ccbbaf9ba65526826bfc3a9c0c4,testori-tori1r8raaqul4j05qtn0t05603mgquxfl8e9p7kcf7smwzcv2hc5rrlq0vket0,testori-tori1436kxs0w2es6xlqpp9rd35e3d0cjnw4sv8j3a7483sgks29jqwgsjscd88
whitelistString = fs.String("teritori-collection-whitelist", "", "whitelist of collections to return")
teritori-dapp/.env
Line 5 in a961899
We handle it though the backend:
teritori-dapp/go/cmd/teritori-dapp-backend/main.go
Line 40 in a961899
New smart contract
It allows to add/remove collections addresses to an on-chain addresses list
It can query this addresses list
New indexes events
It allows to enable/disable a
whitelisted
flag on an item from thecollections
table