Skip to content
This repository has been archived by the owner on Nov 14, 2023. It is now read-only.

feat(subm): 1 BLD request status google sheet #44

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Conversation

dckc
Copy link
Member

@dckc dckc commented Apr 6, 2022

This puts approved 1 BLD requests in a google sheet and shows the transaction that fulfills the ones that have been serviced.

It is run manually using yarn upsert with an .envrc containing credentials for discord, google and referring to google-services-private-key.pem.

Scheduling it to run periodically or on a trigger would be a nice follow-on.

cc @kennyrowe

obsoletes #43

@dckc
Copy link
Member Author

dckc commented Apr 13, 2022

I just got this running on one of our GCP machines to verify that it doesn't rely on anything that's only on my desktop.

};

const messages = await paged(channel.getMessages);
const hasAddr = messages.filter(msg => msg.content.match(/agoric1/));

Choose a reason for hiding this comment

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

need tighter regex match

Copy link
Member Author

Choose a reason for hiding this comment

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

need? really? I think no regex is needed at all; this is just an optimization to avoid processing most messages. The 2 reviewers are the real gating factor.

Choose a reason for hiding this comment

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

true as the extract is lower - i figured it would be possible to copy the same regex up here to tighten down the search

}
}
if (endorsers.length >= quorum) {
const [_, address] = msg.content.match(/(agoric1\S+)/);

Choose a reason for hiding this comment

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

need tighter regex match + bech32 validation

Copy link
Member Author

Choose a reason for hiding this comment

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

it's not clear to me why this is needed either. For a correct request, this is sufficient to pick out the address.

This tool isn't designed to validate requests. It's designed to find requests that have been validated by reviewers.

I guess I can read "need" as "please add ...".

Copy link

@arirubinstein arirubinstein Apr 13, 2022

Choose a reason for hiding this comment

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

we're operating on the value - so this would allow an agoric1"/><whatever"breakgoogleparsingwhoknows}, which we could just not have any exposure to with validation. Since there's manual review currently you could rely on that, but if it were me I would prefer exact control over the shape and content of thing for input validation's sake

Copy link
Member Author

Choose a reason for hiding this comment

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

operating on the value is good justification. thanks.

return data;
};

return freeze({
/** @param { Snowflake } channelID */
channels: channelID => {
return freeze({
Copy link
Member

@dtribble dtribble Apr 13, 2022

Choose a reason for hiding this comment

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

All these freezes would ideally be hardens. I don't know how helpful they are since they are not recursive.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have a certain level of PTSD from trying to use HardenedJS in CLI tools. Perhaps it's less painful now than when I ran into problems a while ago. I can give it a try.

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

Successfully merging this pull request may close these issues.

3 participants