-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Fix/charity #852
Fix/charity #852
Conversation
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.
Thanks so much for doing this! Really appreciate your contributions! I actually was making some changes to this in a draft PR but added my comments here. Note we will want to test that this still works :) Feel free to use the example in the plugin as a way to test it!
const isCharityEnabled = process.env.IS_CHARITABLE === 'true' && isCharitable; | ||
|
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.
can we use runtime.getSetting instead of this check thanks :)
} | ||
|
||
const networkKey = `CHARITY_ADDRESS_${network.toUpperCase()}`; | ||
const charityAddress = process.env[networkKey]; |
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.
same here :)
charityAddress = "0x1234567890123456789012345678901234567890"; | ||
export function getCharityAddress(network: string, isCharitable: boolean = false): string | null { | ||
// Check both environment variable and passed parameter | ||
const isCharityEnabled = process.env.IS_CHARITABLE === 'true' && isCharitable; |
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 funny because I was about to make this nullable :) Great minds!
@@ -0,0 +1,28 @@ | |||
import { elizaLogger } from "@ai16z/eliza"; |
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.
Not sure why these whatsapp changes are included
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.
looks like upstream changes from before idk
IS_CHARITABLE=false # Set to true to enable charity donations | ||
CHARITY_ADDRESS_BASE=0x1234567890123456789012345678901234567890 | ||
CHARITY_ADDRESS_SOL=pWvDXKu6CpbKKvKQkZvDA66hgsTB6X2AgFxksYogHLV | ||
CHARITY_ADDRESS_ETH=0x750EF1D7a0b4Ab1c97B7A623D7917CcEb5ea779C | ||
CHARITY_ADDRESS_ARB=0x1234567890123456789012345678901234567890 | ||
CHARITY_ADDRESS_POL=0x1234567890123456789012345678901234567890 |
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.
Can you please update the BASE address to the ETH address they should be the same.
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.
And arb and poly should be empty for now as they are dummy addresses atm :)
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're removing hardcoded wallets its weird to force projects to give charity
charityAddress = "0x1234567890123456789012345678901234567890"; | ||
} else if (network === "pol") { | ||
charityAddress = "0x1234567890123456789012345678901234567890"; | ||
export function getCharityAddress(network: string, isCharitable: boolean = false): string | null { |
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 will also want to handle what happens when getCharityAddress returns null. I had some changes here for that feel free to take a look and integrate: f7ace73
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.
its a consensus of the group to have charitble as a boolean set to false
will clean this up and re pull |
Relates to:
Wallets in charity hardcoded, i changed this to an env for users to choose if they want to use it or not.
Risks
HIGH
Background
What does this PR do?
fixes hardcoded wallets in main
What kind of change is this?
Documentation changes needed?
Testing
Where should a reviewer start?
Detailed testing steps