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

Expose GetContractState #3161

Merged
merged 3 commits into from
Feb 23, 2024
Merged

Expose GetContractState #3161

merged 3 commits into from
Feb 23, 2024

Conversation

shargon
Copy link
Member

@shargon shargon commented Feb 23, 2024

@shargon shargon requested a review from Jim8y February 23, 2024 13:24
@shargon shargon added the Blocker Issues that are blocking other issues. Check issues details to see what it is blocking. label Feb 23, 2024
@shargon
Copy link
Member Author

shargon commented Feb 23, 2024

This is required now for query the manifest and nef for native contracts

@Jim8y
Copy link
Contributor

Jim8y commented Feb 23, 2024

Has to be public?

@shargon
Copy link
Member Author

shargon commented Feb 23, 2024

Has to be public?

After this was public, the nef, and the manifest

@@ -11,7 +11,7 @@

namespace Neo.Persistence
{
internal class MemoryStoreProvider : IStoreProvider
public class MemoryStoreProvider : IStoreProvider
Copy link
Member Author

Choose a reason for hiding this comment

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

@Jim8y Now that we have two constructors in neoSystem, we need to expose also de provider, otherwise, devpack also fails.

@Jim8y
Copy link
Contributor

Jim8y commented Feb 23, 2024

Has to be public?

After this was public, the nef, and the manifest

What I am saying is why not make it visible to other sub modules, instead of making this one public.

@@ -26,6 +26,7 @@

<ItemGroup>
<InternalsVisibleTo Include="Neo.SmartContract.Testing" />
<InternalsVisibleTo Include="Neo.SmartContract.TestEngine" />
Copy link
Member Author

Choose a reason for hiding this comment

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

Now, the old testEngine require some internals too

@Jim8y
Copy link
Contributor

Jim8y commented Feb 23, 2024

Up to you to decide, but once you make them public, there is no way back, you can not change them at well anymore, and you must maintain them.

@shargon
Copy link
Member Author

shargon commented Feb 23, 2024

Up to you to decide, but once you make them public, there is no way back, you can not change them at well anymore, and you must maintain them.

These are good to be public

@shargon shargon merged commit 66ef246 into master Feb 23, 2024
6 checks passed
@shargon shargon deleted the core-fix-internal branch February 23, 2024 13:42
@roman-khimov roman-khimov added this to the v3.7.0 milestone Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocker Issues that are blocking other issues. Check issues details to see what it is blocking. Waiting for Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants