Skip to content

Commit

Permalink
Testutils bug(s) (#4489)
Browse files Browse the repository at this point in the history
This makes 3 changes:

1. Stops passing `--features testutils` to soroban when building with
`BUILD_TESTS`
2. Starts passing it to librust_stellar_core.a (where it enables the
side-by-side execution feature)
3. Fixes a couple minor typos in the code that was gated by that

The key part here is the first; evidently having `testutils` turned on
on soroban makes the cost structure observably different and thereby
causes divergence during replay. This seems wrong to me, and something
we should track down and eliminate, but in the meantime this at least
(a) gets rid of that divergence and (b) does what we did before I
accidentally started passing `testutils` in the recent soroban-submodule
change (#4456)
  • Loading branch information
sisuresh committed Sep 27, 2024
2 parents 0256109 + 998cf69 commit 2d8d764
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 17 deletions.
14 changes: 4 additions & 10 deletions src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,6 @@ include $(srcdir)/src.mk

noinst_HEADERS = $(SRC_H_FILES)

if BUILD_TESTS
stellar_core_SOURCES = main/StellarCoreVersion.cpp main/XDRFilesSha256.cpp $(SRC_CXX_FILES) $(SRC_TEST_CXX_FILES)
else # !BUILD_TESTS
stellar_core_SOURCES = main/StellarCoreVersion.cpp main/XDRFilesSha256.cpp $(SRC_CXX_FILES)
endif # !BUILD_TESTS

if USE_TRACY
CARGO_FEATURE_TRACY = --features tracy
else
Expand Down Expand Up @@ -187,13 +181,13 @@ $(SOROBAN_LIBS_STAMP): $(wildcard rust/soroban/p*/Cargo.lock) Makefile $(RUST_DE
FEATURE_FLAGS="" ; \
case "$$proto" in \
p21) \
FEATURE_FLAGS="$(CARGO_FEATURE_TESTUTILS)" \
FEATURE_FLAGS="" \
;; \
$(SOROBAN_MAX_PROTOCOL)) \
FEATURE_FLAGS="$(CARGO_FEATURE_TRACY) $(CARGO_FEATURE_NEXT) $(CARGO_FEATURE_TESTUTILS)" \
FEATURE_FLAGS="$(CARGO_FEATURE_TRACY) $(CARGO_FEATURE_NEXT)" \
;; \
*) \
FEATURE_FLAGS="$(CARGO_FEATURE_TRACY) $(CARGO_FEATURE_TESTUTILS)" \
FEATURE_FLAGS="$(CARGO_FEATURE_TRACY)" \
;; \
esac ; \
cd $(abspath $(RUST_BUILD_DIR))/soroban/$$proto && \
Expand Down Expand Up @@ -226,7 +220,7 @@ $(LIBRUST_STELLAR_CORE): $(RUST_HOST_DEPFILES) $(SRC_RUST_FILES) Makefile $(SORO
--$(RUST_PROFILE) \
--locked \
--target-dir $(abspath $(RUST_TARGET_DIR)) \
$(CARGO_FEATURE_TRACY) $(CARGO_FEATURE_NEXT) \
$(CARGO_FEATURE_TRACY) $(CARGO_FEATURE_NEXT) $(CARGO_FEATURE_TESTUTILS) \
-- \
$(ALL_SOROBAN_EXTERN_ARGS) \
$(ALL_SOROBAN_DEPEND_ARGS)
Expand Down
13 changes: 6 additions & 7 deletions src/rust/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -513,9 +513,9 @@ use log::partition::TX;

// We have multiple copies of soroban linked into stellar-core here. This is
// accomplished using an adaptor module -- contract.rs -- mounted multiple times
// into the same outer crate, inside different modules soroban_p21, soroban_p22,
// etc. each with its own local binding for the external crate soroban_env_host.
// The contract.rs module imports soroban_env_host from `super` which means each
// into the same outer crate, inside different modules p21, p22, etc. each with
// its own local binding for the external crate soroban_env_host. The
// contract.rs module imports soroban_env_host from `super` which means each
// instance of it sees a different soroban. This is a bit of a hack and only
// works when the soroban versions all have a compatible _enough_ interface to
// all be called from "the same" contract.rs.
Expand Down Expand Up @@ -774,6 +774,7 @@ mod test_extra_protocol {

use super::*;
use std::hash::Hasher;
use std::str::FromStr;

pub(super) fn maybe_invoke_host_function_again_and_compare_outputs(
res1: &Result<InvokeHostFunctionOutput, Box<dyn std::error::Error>>,
Expand Down Expand Up @@ -845,9 +846,7 @@ mod test_extra_protocol {
) -> Result<(), Box<dyn std::error::Error>> {
match new_protocol {
22 => {
use soroban_p22::contract::{
inplace_modify_cxxbuf_encoded_type, xdr::SorobanResources,
};
use p22::contract::{inplace_modify_cxxbuf_encoded_type, xdr::SorobanResources};
if let Ok(extra) = std::env::var("SOROBAN_TEST_CPU_BUDGET_FACTOR") {
if let Ok(factor) = u32::from_str(&extra) {
inplace_modify_cxxbuf_encoded_type::<SorobanResources>(
Expand Down Expand Up @@ -884,7 +883,7 @@ mod test_extra_protocol {

match new_protocol {
22 => {
use soroban_p22::contract::{
use p22::contract::{
inplace_modify_cxxbuf_encoded_type, xdr::ContractCostParams,
xdr::ContractCostType as CT,
};
Expand Down

0 comments on commit 2d8d764

Please sign in to comment.