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

Fixed bug which caused calling entry points for smart contracts deplo… #4969

Conversation

zajko
Copy link

@zajko zajko commented Nov 20, 2024

…yed as "Transaction::Deploy" (or created in v1.x) to always fail. The fix involves changing the way we fetch entry point data in DataAccessLayer. If entry point is not found under Key::EntryPoint we fallback to see if there is a contract entry for the given contract hash. If a Contract entry exists, we query it's entry_points field.

…yed as "Transaction::Deploy" (or created in v1.x) to always fail. The fix involves changing the way we fetch entry point data in DataAccessLayer. If entry point is not found under Key::EntryPoint we fallback to see if there is a contract entry for the given contract hash. If a Contract entry exists, we query it's `entry_points` field
fn entry_point(&self, request: EntryPointsRequest) -> EntryPointsResult {
let state_hash = request.state_hash();
let query_request = QueryRequest::new(state_hash, request.key(), vec![]);
fn entry_point(&self, request: EntryPointsRequest) -> EntryPointResult {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like if you are going to rename result to singular, you should rename request to singular to match.

Copy link
Collaborator

@EdHastingsCasperAssociation EdHastingsCasperAssociation left a comment

Choose a reason for hiding this comment

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

LGTM, but see comment re: request / response and rename.

}
}
}
}

/// Gets an entry point value.
fn entry_point_exists(&self, request: EntryPointsRequest) -> EntryPointExistsResult {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I acknowledge that it does work to re-use EntryPointsRequest here, but to the best of my knowledge we have kept 1:1 parity between request and response pairs (though I could be wrong). Thus I would think we would do one of the following:

  1. have an EntryPointExistsRequest corresponding to EntryPointExistsResponse to adhere to the request / response isomorphism convention.
  2. instead of adding EntryPointExistsResponse, add a variant to EntryPointResponse to handle the exists result, and modify EntryPointRequest to have a bool existence_check indicating the caller only wants t/f and does not want the entry point instance.

@zajko
Copy link
Author

zajko commented Nov 21, 2024

bors r+

Copy link
Contributor

Build succeeded:

@casperlabs-bors-ng casperlabs-bors-ng bot merged commit 843b19e into casper-network:feat-2.0 Nov 21, 2024
3 checks passed
@devendran-m
Copy link
Contributor

Related issue is #4959

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