Skip to content

Remove MPC validation #13

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Dec 20, 2024
Merged

Remove MPC validation #13

merged 6 commits into from
Dec 20, 2024

Conversation

SurgeCode
Copy link
Member

No description provided.

bh2smith
bh2smith approved these changes Dec 20, 2024
View reviewed changes
Copy link
Member

@bh2smith bh2smith left a comment

Choose a reason for hiding this comment

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

Seems alright.
Not sure about the change from || to && and how its relevant here. There is a TODO that should be removed and an error message that should be changed based on the or and condition change.

Note that this now exposes us to spam because anyone can call the agent.

packages/agent-sdk/src/evm/index.ts
const metadata = JSON.parse(metadataHeader ?? "{}");
const { accountId, evmAddress } = metadata;
if (!accountId || !evmAddress) {
if (!accountId && !evmAddress) {
Copy link
Member

Choose a reason for hiding this comment

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

I think we should still be erroring if only one is provided? Why was this change included here?

Copy link
Member Author

Choose a reason for hiding this comment

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

This can be refined but we don't need to have both to create evm transactions anymore

bh2smith
bh2smith reviewed Dec 20, 2024
View reviewed changes
Copy link
Member

@bh2smith bh2smith left a comment

Choose a reason for hiding this comment

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

Also not that we are not actually validating that the evm Address is an evm address anymore. Before it was indirectly implied by the call to getAddress before.

SurgeCode and others added 3 commits December 20, 2024 12:57
@SurgeCode
Updated tests
a6d6bbe
@SurgeCode @bh2smith
Update packages/agent-sdk/src/evm/index.ts
b16e620 
Co-authored-by: Benjamin Smith <bh2smith@users.noreply.github.com>
@SurgeCode @bh2smith
Update packages/agent-sdk/src/evm/index.ts
f0971b9 
Co-authored-by: Benjamin Smith <bh2smith@users.noreply.github.com>
@SurgeCode
Copy link
Member Author

Seems alright. Not sure about the change from || to && and how its relevant here. There is a TODO that should be removed and an error message that should be changed based on the or and condition change.

Note that this now exposes us to spam because anyone can call the agent.

Also not that we are not actually validating that the evm Address is an evm address anymore. Before it was indirectly implied by the call to getAddress before.

IMO that should be done by the runtime

@SurgeCode SurgeCode merged commit ad51f46 into main Dec 20, 2024
1 check passed
@SurgeCode SurgeCode deleted the remove-mpc-validation branch December 20, 2024 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bh2smith bh2smith bh2smith approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@SurgeCode @bh2smith