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

chore::> Constrain BroadcastDeclare and BroadcastDeployAccount #585

Merged
merged 6 commits into from
Jul 4, 2024

Conversation

PsychoPunkSage
Copy link
Contributor

Here
I implemented:

  1. GetContractClass ::> BroadcastDeclareTxnType interface
  2. GetConstructorCalldata and GetContractAddressSalt ::> BroadcastAddDeployTxnType interface

As these are the best distinguishing factors for objects that implements respective interfaces

@PsychoPunkSage
Copy link
Contributor Author

I have a question...
Shouldn't THIS PART be assigned to BroadcastDeclareTxnType instead of BroadcastAddDeployTxnType.
Cause its a test for DeclareTransaction not AddDeployTxn...

@rianhughes
Copy link
Contributor

rianhughes commented Jul 3, 2024

Shouldn't THIS PART be assigned to BroadcastDeclareTxnType instead of BroadcastAddDeployTxnType.

Yes, you're correct, nice catch!

You'll also need to remove the switch-case logic in the AddDeclareTransaction method. After that you should be able to fix the tests (which will require updating mock_starknet_addDeclareTransaction (since this function is called in the mock tests))

@PsychoPunkSage
Copy link
Contributor Author

Done, Updated everything, ready to be merged!!

@rianhughes rianhughes merged commit 9d42a78 into NethermindEth:main Jul 4, 2024
2 checks passed
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.

2 participants