Mismatching a contract id to an interface produces less than helpful errors.

[kalepail]

What version are you using?

soroban-sdk = "21.3.0"

% stellar --version
stellar 21.2.0 (2c8e512e88d9e29a6b66d63b23f60127675f3c46)
soroban-env 21.2.0 (8809852dcf8489f99407a5ceac12625ee3d14693)
soroban-env interface version 90194313216
stellar-xdr 21.2.0 (9bea881f2057e412fdbb98875841626bf77b4b88)
xdr curr (70180d5e8d9caee9e8645ed8a38c36a8cf403cd9)

And specifically using an Env build from a snapshot.json
let env = Env::from_ledger_snapshot_file("snapshot.json");

What did you do?

I loaded in a SAC contract from an Address that wasn't a SAC. (My bad I know, but an innocent mistake)

let sac = Address::from_string(&String::from_str(
    &env,
    "CDGOXJBEKI3MQDB3J477NN3HAQBDCNK5YYB2ZKAG24US53RXW44QIF6Z",
));
let token_client = token::Client::new(&env, &sac);

I got an error I wasn't sure how to grok

---- test::test stdout ----
thread 'test::test' panicked at /Users/tylervanderhoeven/.cargo/registry/src/index.crates.io-6f17d22bba15001f/soroban-env-host-21.2.0/src/host.rs:768:9:
HostError: Error(WasmVm, MissingValue)

Event log (newest first):
   0: [Diagnostic Event] contract:CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM, topics:[error, Error(WasmVm, MissingValue)], data:"escalating error to panic"
   1: [Diagnostic Event] contract:CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM, topics:[error, Error(WasmVm, MissingValue)], data:["contract call failed", balance, [CAKX2ZMAKMID6PEDSUHBMU4NAHWUIEFXTXHESCSN7IRVG5E4QKAWSGLU]]
   2: [Failed Diagnostic Event (not emitted)] contract:CAKX2ZMAKMID6PEDSUHBMU4NAHWUIEFXTXHESCSN7IRVG5E4QKAWSGLU, topics:[error, Error(WasmVm, MissingValue)], data:["invoking unknown export", balance]
   3: [Diagnostic Event] contract:CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM, topics:[fn_call, Bytes(157d658053103f3c83950e16538d01ed4410b79dce490a4dfa2353749c828169), balance], data:CAKX2ZMAKMID6PEDSUHBMU4NAHWUIEFXTXHESCSN7IRVG5E4QKAWSGLU

What did you expect to see?

A more helpful error. Maybe using words like functions or bad/unknown interface. I didn't understand the "invoking unknown export". What export?

What did you see instead?

Error logs that didn't help much in clueing me into having the wrong interface for the SAC.

[leighmcculloch]

@stellar/contract-committers The error message when a function is called that doesn't exist on the contract could be reworded so it speaks into a larger audiences vocab. Most devs probably won't immediately understand that "WasmVm, MissingValue", or "invoking unknown export", mean they're calling a contract function that doesn't exist.

We could replace "invoking unknown export" with "invoking unknown function, the following functions are available: <list of functions>".

Maybe we could also replace "MissingValue" with "UnknownFunction", or would that be a protocol change?

Wdyt?

[dmkozh]

We could replace "invoking unknown export" with "invoking unknown function, the following functions are available: ".

Yeah, this message is technically correct (because it's possible to e.g. export a variable and that check will pass). But for the sake of simplicity, we can just say that an unknown function is being called.

the following functions are available: ".

That might be tricky to add. I think just pointing at which function is being called and maybe a contract id (if we have it around) should be good enough most of the time. In general, due to the way metering and logging interact, it's hard to build entirely new objects, especially when they're non-trivial and require additional processing (like export lists).

Maybe we could also replace "MissingValue" with "UnknownFunction", or would that be a protocol change?

That's a protocol change as we don't even have UnknownFunction error code. In general, our error space is quite limited (by design) and TBH I don't see much value in it most of the time, besides a few cases where we simply can't attach any message to the error. Maybe we should consider changing the rendering of diagnostic events to highlight the message and its arguments (when present), then either omit the error codes completely (given that message is present), or at least render them as the last and the least important piece of information.

[leighmcculloch]

👍🏻 Finding a balance and improving what we can while limiting the cost of doing so and without making a protocol change sounds good to me. If that means updating the message to "invoking unknown function" and including the contract id and the attempted function name, that sounds great 👏🏻.

[dmkozh]

2: [Failed Diagnostic Event (not emitted)] contract:CAKX2ZMAKMID6PEDSUHBMU4NAHWUIEFXTXHESCSN7IRVG5E4QKAWSGLU, topics:[error, Error(WasmVm, MissingValue)], data:["invoking unknown export", balance]

Actually the necessary pieces are already there (both contract address and the function name), so technically we need to only change the message. But I think this highlights the issue of the diagnostic event rendering; it's not too hard for us to add placeholders to the event message, and then we could render this as something like the following:

"Encountered error in contract CAKX2ZMAKMID6PEDSUHBMU4NAHWUIEFXTXHESCSN7IRVG5E4QKAWSGLU: trying to invoke unknown contract function balance"

Of course that doesn't concern this particular message, I just think we can do better on error presentation in general.