-
Notifications
You must be signed in to change notification settings - Fork 8
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: mod/client referrals #152
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 12a98ba The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
almost there! We should probably also redeploy the example with the new env variable as well
@@ -217,6 +219,7 @@ export default function EditorExample() { | |||
input={getText()} | |||
embeds={getEmbeds()} | |||
api={API_URL} | |||
clientReferralAddress={CLIENT_REFERRAL_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.
needs to be changed in the docs as well
}; | ||
farcaster?: { | ||
fid?: string; | ||
}; | ||
}; | ||
clientReferralAddress?: `0x${string}`; |
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.
should have a comment above describing it further; on which chain? EOA required?
I think we should also call it appEthRewardsAddress
or something instead
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.
Will add a comment. I'm not too attached to the name, but I do prefer referring to the party implementing Mod as the client since app could be ambiguous
packages/core/src/renderer.ts
Outdated
@@ -933,13 +934,27 @@ export class Renderer { | |||
case "SENDETHTRANSACTION": { | |||
const promise = new Promise<void>((resolve) => { | |||
setTimeout(() => { | |||
/* Populate attribution tags */ | |||
const zeroAddress = "0000000000000000000000000000000000000000"; | |||
const modTag = this.manifest.custodyAddress.startsWith("0x") |
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 probably check that this custodyAddress is valid in other ways, such as the length or characters. I have no idea what happens if you overflow the end of txData here but I don't want to find out
Change Summary
ModManifest
'scustodyAddress
fieldTodo: Indexer and claiming site
Merge Checklist