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

Problem: anyone can call request batch txs #166

Merged
merged 2 commits into from
Nov 25, 2022

Conversation

thomas-nguy
Copy link
Collaborator

Solution limit this endpoints to only orchestrators and validators

@thomas-nguy thomas-nguy temporarily deployed to CI November 25, 2022 02:24 Inactive
@thomas-nguy thomas-nguy temporarily deployed to CI November 25, 2022 02:24 Inactive
@thomas-nguy thomas-nguy temporarily deployed to CI November 25, 2022 02:24 Inactive
@thomas-nguy thomas-nguy temporarily deployed to CI November 25, 2022 02:24 Inactive
@thomas-nguy thomas-nguy force-pushed the thomas/request-batch-tx-authorization branch from f3c7726 to 3e3f637 Compare November 25, 2022 03:58
@thomas-nguy thomas-nguy temporarily deployed to CI November 25, 2022 04:29 Inactive
@thomas-nguy thomas-nguy temporarily deployed to CI November 25, 2022 04:29 Inactive
@thomas-nguy thomas-nguy temporarily deployed to CI November 25, 2022 04:29 Inactive
@thomas-nguy thomas-nguy temporarily deployed to CI November 25, 2022 04:29 Inactive
Copy link

@tomtau tomtau left a comment

Choose a reason for hiding this comment

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

likely ok, but not sure how the delegate keys are managed internally.
if it's expected to be increasing, many this could be indexed, so one can just do one lookup k.isAuthorized(msg.Signer) ?

Comment on lines +262 to +263
delegateKeys := k.getDelegateKeys(ctx)
for _, delegateKey := range delegateKeys {
Copy link

Choose a reason for hiding this comment

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

will there be many delegate keys / keep increasing?

Choose a reason for hiding this comment

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

I'm wondering if we should clean up the delegatekeys for every block at BeginBlocker so that we could correctly update the valset in gravity module. I didn't see any method for gravity module to delete delegatekeys.

func BeginBlocker(ctx sdk.Context, k keeper.Keeper) {
cleanupTimedOutBatchTxs(ctx, k)
cleanupTimedOutContractCallTxs(ctx, k)
createSignerSetTxs(ctx, k)
createBatchTxs(ctx, k)
pruneSignerSetTxs(ctx, k)
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah but it should be fine in our case, for the time being, as we are not planning to have many update in our validator set.
It goes along with the ability to update the delegateKeys once set which is a pending issue
#111

@thomas-nguy thomas-nguy merged commit ce79dd8 into v2.0.0-cronos Nov 25, 2022
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