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(connector-ethereum): additional properties to deployContract does not throw error #3507

Open
ashnashahgrover opened this issue Sep 3, 2024 · 2 comments
Assignees
Labels
bug Something isn't working good-first-issue Good for newcomers good-first-issue-400-expert P4 Priority 4: Low Tests Anything related to tests be that automatic or manual, integration or unit, etc.
Milestone

Comments

@ashnashahgrover
Copy link
Contributor

ashnashahgrover commented Sep 3, 2024

Describe the bug

The deployContract function in cactus-plugin-ledger-connector-ethereum takes DeployContractV1Request as an argument, which already declares additional properties as forbidden in the Open API spec as can be seen here:

"DeployContractV1Request": {
        "type": "object",
        "required": ["web3SigningCredential", "contract"],
        **"additionalProperties": false,**
        "properties": {
          "web3SigningCredential": {
            "$ref": "#/components/schemas/Web3SigningCredential"
          },
          "contract": {
            "oneOf": [
              {
                "$ref": "#/components/schemas/ContractJsonDefinition"
              },
              {
                "$ref": "#/components/schemas/ContractKeychainDefinition"
              }
            ],
            "nullable": false
          },
          "constructorArgs": {
            "description": "The list of arguments to pass in to the constructor of the contract being deployed.",
            "type": "array",
            "default": [],
            "items": {}
          },
          "gasConfig": {
            "$ref": "#/components/schemas/GasTransactionConfig"
          },
          "value": {
            "type": "string",
            "description": "Ether balance to send on deployment.",
            "nullable": false
          }
        }
      }

However in various tests that input additional properties, and expect an error to be thrown, the test actually passes.

To Reproduce

Run the test beginning on this line.

The test should throw an error, but it passes without an error.

Expected behavior

The test should throw an error saying additional properties are forbidden.

Logs/Stack traces
Run the test this way, with the fail statement commented out, as that was actually causing the test to fail rather than the additional parameters provided (fake: 4 is the additional parameter):

  test("deployContract with additional parameters should fail", async () => {
    try {
      const result = await apiClient.deployContract({
        contract: {
          contractJSON: HelloWorldContractJson,
        },
        web3SigningCredential: {
          ethAccount: WHALE_ACCOUNT_ADDRESS,
          secret: "",
          type: Web3SigningCredentialType.GethKeychainPassword,
        },
        gas: 1000000,
        fake: 4,
      } as DeployContractV1Request);
      // fail("Expected deployContract call to fail but it succeeded.");
      console.log("deployContract actually returned a response", result);
    } catch (error) {
      console.log("deployContract failed as expected");
    }
  });

And it will return this in the log:

deployContract actually returned a response {
status: 200,
statusText: 'OK',
headers: Object [AxiosHeaders] {
'x-powered-by': 'Express',
'content-type': 'application/json; charset=utf-8',
'content-length': '993',
etag: 'W/"3e1-Cc+on/RU7altdwH6OGOLX2/wWiM"',
date: 'Tue, 03 Sep 2024 01:00:19 GMT',
connection: 'close'
},
config: {
transitional: {
silentJSONParsing: true,
forcedJSONParsing: true,
clarifyTimeoutError: false
},
...

Full log here:
testPassed.txt

Hyperledger Cactus Plugins/Connectors Used

  • Ethereum

Additional context

@ashnashahgrover ashnashahgrover added the bug Something isn't working label Sep 3, 2024
@petermetz
Copy link
Member

@ashnashahgrover Thank you very much, this will help us keep track of the bug!

@petermetz petermetz changed the title test(ethereum): additional properties to deployContract does not throw error fix(connector-ethereum): additional properties to deployContract does not throw error Sep 4, 2024
@petermetz petermetz added good-first-issue Good for newcomers good-first-issue-400-expert Tests Anything related to tests be that automatic or manual, integration or unit, etc. P2 Priority 2: High labels Sep 4, 2024
@petermetz petermetz added this to the v2.0.0 milestone Sep 4, 2024
@petermetz petermetz added P4 Priority 4: Low and removed P2 Priority 2: High labels Sep 9, 2024
@petermetz petermetz modified the milestones: v2.0.0, vT.B.D Sep 9, 2024
@jagpreetsinghsasan jagpreetsinghsasan modified the milestones: vT.B.D, v2.0.0 Sep 18, 2024
@jagpreetsinghsasan
Copy link
Contributor

From the conversation here: #3496 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good-first-issue Good for newcomers good-first-issue-400-expert P4 Priority 4: Low Tests Anything related to tests be that automatic or manual, integration or unit, etc.
Projects
Status: In Progress
Development

No branches or pull requests

4 participants