-
Notifications
You must be signed in to change notification settings - Fork 77
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
passing e2e tests #855
passing e2e tests #855
Conversation
will need to get the hello_world and increment contracts on soroban-examples updated to use the latest soroban-sdk with latest env-host versions, when running the tests using contracts from soroban-examples/main, getting preflight errors on the contract interface version
|
@sreuland are you still working on this pr ? |
yes, this isn't ready to merge yet as it's building system-test with a reference to soroban-examples/e2e_version_bump to build contracts with the version of soroban-sdk used to match that of latest core to fix the earlier reported pre-release version number does not match error. Ideally need to get a tag or next release version of soroban-examples to use as reference instead? beyond that, tests are not passing currently, when running cli and rpc server built from soroban-tools/main, seeing what looks like a regression in
|
@sreuland I just pushed some fixes. I used https://github.com/stellar/soroban-tools/blob/main/cmd/soroban-rpc/internal/methods/simulate_transaction.go#L21 as a reference and turns out that all but latestLedger can be omitted. Also noticed that the Cost type had fields as strings but the go was u64 so I fixed that too. @tsachiherman @paulbellamy @2opremio One question about the omittable fields. Does it make sense for them to be the defaults for those types? Or should we make all of the types Options to reflect the fact that the field was not provided? |
making progress, @willemneal fixed the json errors in cli on simulation response, the tests are now progressing to
looks like the cli may have included auth entries as part of |
My guess is that the format is wrong for some reason. This is where it fails: let needs_auth_recording = invoke_hf_op.auth.is_empty();
if needs_auth_recording {
host.switch_to_recording_auth(true)
.context("cannot switch auth to recording mode")?;
} else {
host.set_authorization_entries(invoke_hf_op.auth.to_vec())
.context("cannot set authorization entries")?;
} @willemneal / @paulbellamy can you take a look? |
Happy to see the error contexts being helpful :) , the |
Are diagnostic events enabled? That error should have a message tied to it (I'm guessing it's one of these two errors). |
during the test case, RPC server ingestion is configured with captive core having those errors you referenced, they are related to ledger entry or signature type expirations? This is occurring during a test on a new standalone network where the contract is installed successfully and then within a couple seconds afterwards, cli is trying to deploy the prior installed contract and [getting that preflight error about 'cannot set authorization entries'], (https://github.com/stellar/soroban-tools/actions/runs/6005737037/job/16289022356?pr=855#step:8:1280), so, it would seem like expiration hasn't had time yet. Where does |
@sisuresh Uhm, maybe we are not printing the errors properly? How are they supposed to be printed? This is how we do it (we use debug printing) https://github.com/stellar/soroban-tools/blob/main/cmd/soroban-rpc/lib/preflight/src/lib.rs#L237
Ever since #878 , the |
Here's an example of an error diagnostic event in pretty-printed xdr
|
In core we use https://github.com/stellar/stellar-core/blob/a4d623048d166885b6891a003574ea0b08d64e17/src/rust/src/contract.rs#L194 which prints as
|
I was referring to the printout of the error you were referring to at #855 (comment) which is a Host error, right? I am surprised that debug printouts don't include the error text (e.g. |
Ahh, after reading the code it seems like the message is only propagated to the diagnostic events. So, yeah, we should make sure we print them on error (@willemneal in Thanks @sisuresh |
@willemneal , with #906 merged into main, was expecting to see more verbose error output from cli when
Should diagnostic events from the sim error be shown now? |
@sreuland I took the liberty to merge this PR since it contained fixes we need in main (namely, making the CLI compatible with the missing simulateTransaction response fields). Sorry if I overstepped. Please create a new PR. Thanks! |
What
bump versions used by e2e
Why
sync up component refs to all have same xdr versions
Known limitations
need to get.new release version of js-soroban-client with the same xdr bump, currently using a gh ref to the pending branch