Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Failing "test_protocol_config_rpc" nayduck test #8967

Closed
aborg-dev opened this issue Apr 24, 2023 · 1 comment
Closed

Failing "test_protocol_config_rpc" nayduck test #8967

aborg-dev opened this issue Apr 24, 2023 · 1 comment
Assignees
@aborg-dev
Labels
A-contract-runtime Area: contract compilation and execution, virtual machines, etc

Comments

@aborg-dev
Copy link
Contributor

After the introduction of non-trivial Compute Costs for storage (#8924), the nayduck test "tests::nearcore::rpc_nodes::test_protocol_config_rpc" started to fail with a mismatch:

storage_write_base: ParameterCost { gas: 64196736000, compute: 64196736000 }
storage_write_base: ParameterCost { gas: 64196736000, compute: 200000000000 }

At this assert:

nearcore/integration-tests/src/tests/nearcore/rpc_nodes.rs

Lines 261 to 265 in fdad133

// compare struct used by runtime
assert_eq!(
RuntimeConfig::from(config_response.config_view.runtime_config),
latest_runtime_config.as_ref().clone()
);

This is because we don't expose the actual compute costs over RPC right now, and when the config is constructed from the RPC response, the compute cost is assumed to be equal to the gas cost.

I've been working on removing this lossy conversion: #8763

CC @jakmeier , @Longarithm
@aborg-dev aborg-dev added the A-contract-runtime Area: contract compilation and execution, virtual machines, etc label Apr 24, 2023
@aborg-dev aborg-dev mentioned this issue Apr 24, 2023
Merged
@aborg-dev aborg-dev self-assigned this Apr 24, 2023
near-bulldozer bot pushed a commit that referenced this issue Apr 25, 2023
@aborg-dev
cleanup: Removing RuntimeConfigView -> RuntimeConfig (#8763)
d752d74 
In #8426 this conversion became lossy due to the introduction of Compute Costs. This raised the question of whether this conversion is needed at all. I've found one place in the Indexer code that uses it and replaced it with a direct lookup for config from `RuntimeConfigStore`. As a part of this I've also simplified some code in the indexer.

This should fix the failing nayduck test #8967
@aborg-dev
Copy link
Contributor Author

This should be resolved by #8763.

nikurt pushed a commit that referenced this issue Apr 25, 2023
@aborg-dev @nikurt
cleanup: Removing RuntimeConfigView -> RuntimeConfig (#8763)
6bc3dc9 
In #8426 this conversion became lossy due to the introduction of Compute Costs. This raised the question of whether this conversion is needed at all. I've found one place in the Indexer code that uses it and replaced it with a direct lookup for config from `RuntimeConfigStore`. As a part of this I've also simplified some code in the indexer.

This should fix the failing nayduck test #8967
nikurt pushed a commit that referenced this issue Apr 28, 2023
@aborg-dev @nikurt
cleanup: Removing RuntimeConfigView -> RuntimeConfig (#8763)
181dd89 
In #8426 this conversion became lossy due to the introduction of Compute Costs. This raised the question of whether this conversion is needed at all. I've found one place in the Indexer code that uses it and replaced it with a direct lookup for config from `RuntimeConfigStore`. As a part of this I've also simplified some code in the indexer.

This should fix the failing nayduck test #8967
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@aborg-dev aborg-dev

Labels
A-contract-runtime Area: contract compilation and execution, virtual machines, etc
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@aborg-dev