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

fix(plugin-ledger-connector): Some of the plugin-ledger-connectors are using wrong Interface Types in their class definitions #1445

Closed
m-courtin opened this issue Oct 7, 2021 · 6 comments · Fixed by #1457
Assignees
Labels
bug Something isn't working

Comments

@m-courtin
Copy link
Contributor

Describe the bug

Some of the plugin-ledger-connectors are using wrong Interface Types in their class definitions where the types for the deployContract() and the transact() functions are getting defined.

To Reproduce

In plugin-ledger-connector-quorum for example the class definition needs to implement "IPluginLedgerConnector<
DeployContractSolidityBytecodeV1Request, DeployContractSolidityBytecodeV1Response,
RunTransactionRequest, RunTransactionResponse >" but the deployContract() is using DeployContractSolidityBytecodeV1Request and RunTransactionResponse instead of the interface definition pair DeployContractSolidityBytecodeV1Request, DeployContractSolidityBytecodeV1Response.

Similar is the case for the besu connector.

Expected behavior

For all the connector class definitions the implementation needs to follow their interface definition and need to match in consistent way for all the connectors.

@m-courtin m-courtin added the bug Something isn't working label Oct 7, 2021
@petermetz
Copy link
Contributor

Research needs to be done on why the compiler is okay with this and if there's any configuration parameter that would make it so that it does error out. Enforcing these things manually will always be a doomed effort so ideally we'd somehow find a way to have the CI do the heavy lifting instead.

@m-courtin
Copy link
Contributor Author

Totally agree on this: The compiler should throw error to show to the outside that the required interface type and the implemented type are not matching

@m-courtin
Copy link
Contributor Author

m-courtin commented Oct 8, 2021

@petermetz: Can you please assign this issue to me

@m-courtin
Copy link
Contributor Author

According to my suggestion I will create a branch named "NOT_fixed_1445_for_reproduction_DONT_UPDATE" and a branch for the fix that we can have as sonn as possible a fixed version and in parallel can do researches on the un-fixed branch to find out why the linter / compiler is not complaining

@petermetz
Copy link
Contributor

@m-courtin Sounds good to me. My only nit: keep branches all lower-case if possible. (some git tooling/operating systems do not take kindly to certain edge cases that stem from mixed-case branch names)

@m-courtin
Copy link
Contributor Author

@petermetz Ok fine, I'll use lower-case only for my branch names

m-courtin added a commit to m-courtin/cactus that referenced this issue Oct 15, 2021
For all the connector class definitions the implementation are fixed to
follow their interface definition and to match in consistent way for all
the connectors

Closes: hyperledger-cacti#1445
Signed-off-by: Michael Courtin <michael.courtin@accenture.com>
m-courtin added a commit to m-courtin/cactus that referenced this issue Oct 15, 2021
Closes: hyperledger-cacti#1445
Signed-off-by: Michael Courtin <michael.courtin@accenture.com>
m-courtin added a commit to m-courtin/cactus that referenced this issue Oct 19, 2021
For all the connector class definitions the implementation are fixed to
follow their interface definition and to match in consistent way for all
the connectors

Closes: hyperledger-cacti#1445
Signed-off-by: Michael Courtin <michael.courtin@accenture.com>
m-courtin added a commit to m-courtin/cactus that referenced this issue Oct 19, 2021
Closes: hyperledger-cacti#1445
Signed-off-by: Michael Courtin <michael.courtin@accenture.com>
m-courtin added a commit to m-courtin/cactus that referenced this issue Oct 19, 2021
For all the connector class definitions the implementation are fixed to
follow their interface definition and to match in consistent way for all
the connectors

Closes: hyperledger-cacti#1445
Signed-off-by: Michael Courtin <michael.courtin@accenture.com>
m-courtin added a commit to m-courtin/cactus that referenced this issue Oct 22, 2021
For all the connector class definitions the implementation are fixed to
follow their interface definition and to match in consistent way for all
the connectors

Closes: hyperledger-cacti#1445
Signed-off-by: Michael Courtin <michael.courtin@accenture.com>
m-courtin added a commit to m-courtin/cactus that referenced this issue Oct 25, 2021
For all the connector class definitions the implementation are fixed to
follow their interface definition and to match in consistent way for all
the connectors

Closes: hyperledger-cacti#1445
Signed-off-by: Michael Courtin <michael.courtin@accenture.com>
m-courtin added a commit to m-courtin/cactus that referenced this issue Oct 26, 2021
For all the connector class definitions the implementation are fixed to
follow their interface definition and to match in consistent way for all
the connectors

Closes: hyperledger-cacti#1445
Signed-off-by: Michael Courtin <michael.courtin@accenture.com>
m-courtin added a commit to m-courtin/cactus that referenced this issue Oct 26, 2021
For all the connector class definitions the implementation are fixed to
follow their interface definition and to match in consistent way for all
the connectors

Closes: hyperledger-cacti#1445
Signed-off-by: Michael Courtin <michael.courtin@accenture.com>
m-courtin added a commit to m-courtin/cactus that referenced this issue Oct 27, 2021
For all the connector class definitions the implementation are fixed to
follow their interface definition and to match in consistent way for all
the connectors

Closes: hyperledger-cacti#1445
Signed-off-by: Michael Courtin <michael.courtin@accenture.com>
m-courtin added a commit to m-courtin/cactus that referenced this issue Oct 28, 2021
For all the connector class definitions the implementation are fixed to
follow their interface definition and to match in consistent way for all
the connectors

Closes: hyperledger-cacti#1445
Signed-off-by: Michael Courtin <michael.courtin@accenture.com>
m-courtin added a commit to m-courtin/cactus that referenced this issue Oct 29, 2021
For all the connector class definitions the implementation are fixed to
follow their interface definition and to match in consistent way for all
the connectors

Closes: hyperledger-cacti#1445
Signed-off-by: Michael Courtin <michael.courtin@accenture.com>
m-courtin added a commit to m-courtin/cactus that referenced this issue Nov 3, 2021
For all the connector class definitions the implementation are fixed to
follow their interface definition and to match in consistent way for all
the connectors

Closes: hyperledger-cacti#1445
Signed-off-by: Michael Courtin <michael.courtin@accenture.com>
petermetz pushed a commit that referenced this issue Nov 4, 2021
For all the connector class definitions the implementation are fixed to
follow their interface definition and to match in consistent way for all
the connectors

Closes: #1445
Signed-off-by: Michael Courtin <michael.courtin@accenture.com>
RafaelAPB pushed a commit to RafaelAPB/blockchain-integration-framework that referenced this issue Mar 9, 2022
For all the connector class definitions the implementation are fixed to
follow their interface definition and to match in consistent way for all
the connectors

Closes: hyperledger-cacti#1445
Signed-off-by: Michael Courtin <michael.courtin@accenture.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants