[SC] Local call log serialization

[rowandh]

See #661.

[rowandh]

@mrtpain would appreciate your input into whether this resolves #661!

[fassadlr]

@rowandh it looks good. Just need @mrtpain to comment 👍

Also, I see the windows build was cancelled?

[rowandh]

Strange, looks like the build died. Will kick it off again.

[fassadlr]

Merge if CI passes

[mrtpain]

Pulled down and tested locally, only remaining thing I noticed is return values. A method returning bool returns "return": "00" for false or "return": "01" for true. Didn't check other response types. cc @rowandh @fassadlr Thanks for the quick turnaround on this

[rowandh]

@mrtpain the return value should be fixed now, could you please check once more and confirm?

[mrtpain]

@rowandh 🥳 thanks all is good now 👍

[mrtpain]

Actually, seems Address and Uint256 are returning {} from the return property. I don't have anything using UInt128 but guessing that too.

[rowandh]

@mrtpain should be fixed now

[mrtpain]

Thanks @rowandh all good now.

Somewhat related: recent versions of FN output the event in logs. This is helpful to have so clients don't need to deserialize topics, but I wonder, what would happen if I output a contract log that had an Event property already?

"log": {
    "event": "CollectMiningRewardsLog",
    "miner": "PPQdeXdjWDBzVLUjgWwi4mFP4Y1mhuNcRu",
    "amount": "2541009128850074"
}

Currently we are deserializing the first topic to get our event names. We can remove a package dependency if we can rely on the log.event property but wondering about the potential conflict outlined above.

[rowandh]

Thanks @rowandh all good now.

Somewhat related: recent versions of FN output the event in logs. This is helpful to have so clients don't need to deserialize topics, but I wonder, what would happen if I output a contract log that had an Event property already?

"log": {
    "event": "CollectMiningRewardsLog",
    "miner": "PPQdeXdjWDBzVLUjgWwi4mFP4Y1mhuNcRu",
    "amount": "2541009128850074"
}

Currently we are deserializing the first topic to get our event names. We can remove a package dependency if we can rely on the log.event property but wondering about the potential conflict outlined above.

Good point, a log property named Event would replace the event name. We should be nesting the actual log data, something like this:

"log": {
    "event": "CollectMiningRewardsLog",
    "data": {
        "miner": "PPQdeXdjWDBzVLUjgWwi4mFP4Y1mhuNcRu",
        "amount": "2541009128850074"
   }
}

[mrtpain]

On another note, @rowandh, transactions with internal CRS transfers I believe either aren't implemented fully (intentionally) or may have a bug.

We are able to successfully broadcast transactions that return errors during a local call. These contract errors are pointing to a 0 CRS amount in Message.Value.

https://github.com/Opdex/opdex-v1-core/blob/main/src/Contracts/Routers/OpdexRouter.cs#L228

That line is throwing during local calls but the same payload sent for an actual transaction is successful. (Note that is a public method but in our scenario of calling Router.AddLiquidity it is being called from within other methods).

Along with the error that we receive in the local call response, we can also see the list of internal transfers showing a 0 amount.

[drmathias]

@rowandh We're getting a 500 response making a local call to this method, when running the full node built from this branch. The request body is below.

{
  "walletName": "{{wallet_name}}",
  "accountName": "{{wallet_account}}",
  "contractAddress": "{{market_deployer}}",
  "methodName": "CreateStandardMarket",
  "amount": "0",
  "feeAmount": ".001",
  "password": "{{wallet_password}}",
  "gasPrice": 100,
  "gasLimit": 250000,
  "sender": "{{wallet_address}}",
  "parameters": [
    "9#{{wallet_address}}", // market owner
    "5#3", // fee
    "1#True", // authPoolCreators
    "1#True", // authProviders
    "1#False", // authTraders
    "1#True" // enableMarketFee
  ]
}

Had a go at debugging locally and it seems to be failing to map the log responses.

[YakupIpek]

@rowandh We're getting a 500 response making a local call to this method, when running the full node built from this branch. The request body is below.

{
  "walletName": "{{wallet_name}}",
  "accountName": "{{wallet_account}}",
  "contractAddress": "{{market_deployer}}",
  "methodName": "CreateStandardMarket",
  "amount": "0",
  "feeAmount": ".001",
  "password": "{{wallet_password}}",
  "gasPrice": 100,
  "gasLimit": 250000,
  "sender": "{{wallet_address}}",
  "parameters": [
    "9#{{wallet_address}}", // market owner
    "5#3", // fee
    "1#True", // authPoolCreators
    "1#True", // authProviders
    "1#False", // authTraders
    "1#True" // enableMarketFee
  ]
}

Had a go at debugging locally and it seems to be failing to map the log responses.

Is this on testnet, mainnet or in localchain(devmode) ? Can you be sure that wallet_address is belong to one of your wallet addresses ?

[mrtpain]

@YakupIpek

@rowandh We're getting a 500 response making a local call to this method, when running the full node built from this branch. The request body is below.

{
  "walletName": "{{wallet_name}}",
  "accountName": "{{wallet_account}}",
  "contractAddress": "{{market_deployer}}",
  "methodName": "CreateStandardMarket",
  "amount": "0",
  "feeAmount": ".001",
  "password": "{{wallet_password}}",
  "gasPrice": 100,
  "gasLimit": 250000,
  "sender": "{{wallet_address}}",
  "parameters": [
    "9#{{wallet_address}}", // market owner
    "5#3", // fee
    "1#True", // authPoolCreators
    "1#True", // authProviders
    "1#False", // authTraders
    "1#True" // enableMarketFee
  ]
}

Had a go at debugging locally and it seems to be failing to map the log responses.

Is this on testnet, mainnet or in localchain(devmode) ? Can you be sure that wallet_address is belong to one of your wallet addresses ?

This is our local network (devnet) running from this branch. Our payload is all correct.

We can switch the endooint from smartcontracts/local-call to smartcontractwallet/call and it works fine. Also we can change just the parameters and method to CreateStakingMarket method with a local call and it works as expected.

So far it seems that it's a problem with something happening in this method specifically that is uncaught as the FN returns 500 error with no body, I do not think the error is getting caught but throwing all the way out.

Also local calls that involve CRS transfers and use of Message.Value in contract always have it as 0 and the local call tx returns failures due to it.

[rowandh]

Will merge these changes in to release/1.0.9.6 and address the new issues with separate PRs.

[rowandh]

f9b3e80 resolves zero amount local calls (#662 (comment))
ae7099c resolves the issue deserializing addresses (#662 (comment))
f7f278a resolves nesting event data (#662 (comment))

[mrtpain]

These changes worked for us and are in use in our Dev and public Testnet environments 🙌

[fassadlr]

@rowandh I believe this fix was merged to release/1.1.1.0... can you confirm? If so it was also then merge to 1.2 already.