filecoin-saturn / contracts

contracts
6 stars 0 forks source link

Implement revoke admin cli support #83

Closed joaosa closed 4 months ago

joaosa commented 4 months ago

We couldn't really remove admins from the cli, so this should fix that. I wanted to cleanup the code a little bit, but I figured it would be more important to ship this rather than adjust existing functionality for code reuse.

joaosa commented 4 months ago

Other than logging already suggested, this looks good. My rust knowledge is minimal for now, so I might be missing things.

Should there be a test for granting and revoking admin?

Thank you! I agree we should have this. I will have to learn how to use the test tooling to pull it off (which is probably a good thing anyway). Should it be a necessary to merge the PR?

joaosa commented 4 months ago

@travis @gammazero

I should also note that as things are, it's not possible to run integration tests locally as we're missing three secrets and we have to rely on CI. However, it's possible to troubleshoot what's going on with all these invocations on calibrationnet. Here's an example of the last CI run with all the integration test transactions (you'll need the abi.json to decode the messages).