Skip to content
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

Work in progress, case sensitivity is going to be tricky #171

Draft
wants to merge 45 commits into
base: main
Choose a base branch
from

Conversation

johnmark13
Copy link
Contributor

WIP

Copy link
Member

@aahna-ashina aahna-ashina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be working well 😄
Also remember to add the script to a GitHub workflow.

scripts/config.js Outdated Show resolved Hide resolved
Comment on lines -7721 to +7723
{"action":{"currency":{"chainId":"1","tokenAddress":"0x333A4823466879eeF910A04D473505da62142069","type":"EVM"},"id":"BtlDOgLvfl76rb1jGwi1oQ","payoutAddress":"0x4e3072f7b5c075ea5fdeb423da95312c4b99dc22","type":"SET_PAYOUT_ADDRESS"},"ledgerTimestamp":1652508453019,"uuid":"2FArHEDBcgKBp44XNOpotQ","version":"1"}
{"action":{"currency":{"chainId":"1","tokenAddress":"0x333A4823466879eeF910A04D473505da62142069","type":"EVM"},"id":"LMUEWiZux2Q8iAcKKy8gGg","payoutAddress":"0x4e3072f7b5c075ea5fdeb423da95312c4b99dc22","type":"SET_PAYOUT_ADDRESS"},"ledgerTimestamp":1652508458077,"uuid":"Q1AmEyAVEg4P5cxyb8ejIA","version":"1"}
{"action":{"currency":{"chainId":"1","tokenAddress":"0x333A4823466879eeF910A04D473505da62142069","type":"EVM"},"id":"o4RCyNUm0AM3DF6NqrRLnQ","payoutAddress":"0x4e3072f7b5c075ea5fdeb423da95312c4b99dc22","type":"SET_PAYOUT_ADDRESS"},"ledgerTimestamp":1652508470072,"uuid":"8uRBXGtmSlM0WynJgnCkQw","version":"1"}
{"action":{"currency":{"chainId":"1","tokenAddress":"0x333A4823466879eeF910A04D473505da62142069","type":"EVM"},"id":"BtlDOgLvfl76rb1jGwi1oQ","payoutAddress":"0x4e3072f7b5C075EA5FdEb423DA95312C4B99dc22","type":"SET_PAYOUT_ADDRESS"},"ledgerTimestamp":1652508453019,"uuid":"2FArHEDBcgKBp44XNOpotQ","version":"1"}
{"action":{"currency":{"chainId":"1","tokenAddress":"0x333A4823466879eeF910A04D473505da62142069","type":"EVM"},"id":"LMUEWiZux2Q8iAcKKy8gGg","payoutAddress":"0x4e3072f7b5C075EA5FdEb423DA95312C4B99dc22","type":"SET_PAYOUT_ADDRESS"},"ledgerTimestamp":1652508458077,"uuid":"Q1AmEyAVEg4P5cxyb8ejIA","version":"1"}
{"action":{"currency":{"chainId":"1","tokenAddress":"0x333A4823466879eeF910A04D473505da62142069","type":"EVM"},"id":"o4RCyNUm0AM3DF6NqrRLnQ","payoutAddress":"0x4e3072f7b5C075EA5FdEb423DA95312C4B99dc22","type":"SET_PAYOUT_ADDRESS"},"ledgerTimestamp":1652508470072,"uuid":"8uRBXGtmSlM0WynJgnCkQw","version":"1"}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnmark13 The change from lower-case to checksummed is probably because you manually ran sourcecred load before running yarn update-ledger-from-chain? It makes it difficult to review the changes, so can we make a PR without these upper-case/lower-case diffs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, we definitely cannot easilly split to multiple PRs. I wonder if SourceCred automatically converts the case to checksummed when setting the payout address, and we have them in lower case because of that script you ran before? If we let the ledger hold checksummed addresses it won't update them after the first time making PRs more simple.

scripts/discord-utils.js Outdated Show resolved Hide resolved

console.log(`Setting payout address for GitHub for passport ${passport_id} - gitHubUsername ${dUsername}`)

ledgerManager.ledger.setPayoutAddress(baseIdentityId, owner_address ,chainId, tokenAddress);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ledgerManager.ledger.setPayoutAddress(baseIdentityId, owner_address ,chainId, tokenAddress);
ledgerManager.ledger.setPayoutAddress(baseIdentityId, owner_address, chainId, tokenAddress);

scripts/github-utils.js Outdated Show resolved Hide resolved

const discourseFile = await readCsv(FILE_DISCOURSE_CSV);

console.info(`Read Citizen Disource data from Github`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
console.info(`Read Citizen Disource data from Github`);
console.info(`Read Citizen Discourse data from Github`);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants