Disallow array parameters in JSONRPC methods

[2opremio]

What

Disallow array parameters in JSONRPC methods

Why

Close #13 Close #40

Do not merge until we want to cut a protocol 21 release

[2opremio]

@Shaptic it seems like javascript is still using array parameters. Could that be the case?

[mollykarcher]

@Shaptic it seems like javascript is still using array parameters. Could that be the case?

It looks like it was updated, so you just need to update the system tests to reference the most recent SDK release

[2opremio]

Uhm, it seems the CLI doesn't like it either ...

soroban cli install of example contract soroban_hello_world_contract.wasm had error 1

[2opremio]

@willemneal Could it be that the CLI uses array arguments in JSONRPC calls?

[willemneal]

Here are some of the jsonrpc requests from a trace from the CLI deploying a contract, which all use the array for "params"

[
  {
    "jsonrpc": "2.0",
    "id": 0,
    "method": "getLedgerEntries",
    "params": [
      [
        "AAAAAAAAAADh9C8tjVls3DqO4pComSAu7Uowvdz14Q1ANPs6kFTv6A=="
      ]
    ]
  },
  {
    "jsonrpc": "2.0",
    "id": 0,
    "method": "getTransaction",
    "params": [
      "303066cd68a6e8a669ca3fc22ca60f07011f9c99c4b73c046ca6e72818d207b5"
    ]
  },
  {
    "jsonrpc": "2.0",
    "id": 0,
    "method": "simulateTransaction",
    "params": {
      "transaction": "AAAAAgAAAADh9C8tjVls3DqO4pComSAu7Uowvdz14Q1ANPs6kFTv6AAAAGQAAADOAAAABwAAAAAAAAAAAAAAAQAAAAAAAAAYAAAAAQAAAAAAAAAAAAAAAOH0Ly2NWWzcOo7ikKiZIC7tSjC93PXhDUA0+zqQVO/oTPSTOwMvaooNPLS9FedDzC5S3DQ6BNZLYmDtS17OKIoAAAAAgPljDahC0uEgl8NOZZOM4yWQttNLhzyz5tyKYt0bA5YAAAAAAAAAAAAAAAA="
    }
  },
  {
    "jsonrpc": "2.0",
    "id": 0,
    "method": "sendTransaction",
    "params": [
      "AAAAAgAAAADh9C8tjVls3DqO4pComSAu7Uowvdz14Q1ANPs6kFTv6AAD2uEAAADOAAAABwAAAAAAAAAAAAAAAQAAAAAAAAAYAAAAAQAAAAAAAAAAAAAAAOH0Ly2NWWzcOo7ikKiZIC7tSjC93PXhDUA0+zqQVO/oTPSTOwMvaooNPLS9FedDzC5S3DQ6BNZLYmDtS17OKIoAAAAAgPljDahC0uEgl8NOZZOM4yWQttNLhzyz5tyKYt0bA5YAAAABAAAAAAAAAAEAAAAAAAAAAAAAAADh9C8tjVls3DqO4pComSAu7Uowvdz14Q1ANPs6kFTv6Ez0kzsDL2qKDTy0vRXnQ8wuUtw0OgTWS2Jg7UteziiKAAAAAID5Yw2oQtLhIJfDTmWTjOMlkLbTS4c8s+bcimLdGwOWAAAAAAAAAAEAAAAAAAAAAQAAAAeA+WMNqELS4SCXw05lk4zjJZC200uHPLPm3Ipi3RsDlgAAAAEAAAAGAAAAAXmbAq7ocZCaTIPZo0Kjumf6ulOoMG8FEPT+i5O+lw+/AAAAFAAAAAEALqtnAAAXDAAAAGgAAAAAAANZxAAAAAGQVO/oAAAAQBFkNvKWhKrv0JXUgfELdpRjXtebp4b6NU8yDljeRvNGhtIwPz0lLSXlLctKmSQMYQ7nN3smlMnpk3WsxS3CGAc="
    ]
  }
]

Looking at the rust crate it uses rpc_params![] which returns an array defined here: https://github.com/paritytech/jsonrpsee/blob/87a37a719a378b0700f519beeee3c21f95bc6495/core/src/client/mod.rs#L216

[2opremio]

Ahh, it's defined in the rpc-client crate. I will try to fix it.

[2opremio]

I think we first need to update the CLI code to use the object parameters. I will create a separate PR for that

[leighmcculloch]

Replying to #125 (comment):

Also, it's a good idea to release an updated CLI version first, to allow users to upgrade before disallowing array parameters for good.

I agree with @2opremio it'd be ideal to release the CLI and for there to be a period of time where the breakage is not experienced so as to reduce it's impact and give folks time to upgrade.

When we planned #121 for this upcoming release we thought that we had already updated every client that was using arrays weeks/months ago, meaning there was plenty of time to pull the plug on arrays. But it'll be pretty rough to pull the plug now if we're only providing a week or two, and we break all existing users.

Maybe that's not a big deal if there's other necessary breaking changes already in play.

[2opremio]

we thought that we had already updated every client that was using arrays weeks/months ago

I thought so too 🤷

[2opremio]

@willemneal will you let me know once the CLI is updated to use the rpc client create version 20.3.5? That should unblock this PR.

Thanks!

[willemneal]

@2opremio Here is the PR to update the deps: stellar/stellar-cli#1281