Skip to content
This repository was archived by the owner on Aug 27, 2025. It is now read-only.

[master] (fix) ZIL-5144: Don't send Scilla contract creation to ARCHIVAL_SEND_…#3530

Merged
rrw-zilliqa merged 1 commit into
masterfrom
ZIL-5144_scilla_contract_creation
Mar 31, 2023
Merged

[master] (fix) ZIL-5144: Don't send Scilla contract creation to ARCHIVAL_SEND_…#3530
rrw-zilliqa merged 1 commit into
masterfrom
ZIL-5144_scilla_contract_creation

Conversation

@rrw-zilliqa
Copy link
Copy Markdown
Contributor

…SHARD unless you are actually a lookup or archival node.

Description

Fix ZIL-5144.

Backward Compatibility

  • This is not a breaking change
  • This is a breaking change

Review Suggestion

Check that the logic is correct - please be careful; this is my first PR which touches actual logic and I'm far from sure I have it right - the only evidence I have is traces and the fact that my trivial deploy-a-Scilla-contract test works with this patch, and fails without it.

…SHARD unless you are actually a lookup or archival node.
@github-actions github-actions Bot changed the title (fix) ZIL-5144: Don't send Scilla contract creation to ARCHIVAL_SEND_… [master] (fix) ZIL-5144: Don't send Scilla contract creation to ARCHIVAL_SEND_… Mar 29, 2023
@valeryz
Copy link
Copy Markdown
Contributor

valeryz commented Mar 30, 2023

The purpose of sending to shards rather than DS is to scale them. contract creation, esp in Scilla should be so much more rare than contract calls that an advantage to scaling it through sharding almost doesn't exist.

At some point, we had unified this among EVM and Scilla, and had sent all creation to DS. In the current codebase you are fixing, it seems it is no longer the case (don't know when it reverted back).

Anyhow, the change LGTM, but FWIW sending them to shards is not necessary at all.

@rrw-zilliqa rrw-zilliqa merged commit cb8862a into master Mar 31, 2023
@rrw-zilliqa rrw-zilliqa deleted the ZIL-5144_scilla_contract_creation branch March 31, 2023 15:30
@rrw-zilliqa
Copy link
Copy Markdown
Contributor Author

Merged 3530 on the basis that we can then decide to merge 3532 over it - before I OK that one, I want to see it pass some tests. Before that, I want to be able to run some tests! Sorry @valeryz ...

JamesHinshelwood pushed a commit that referenced this pull request Apr 4, 2023
…SHARD unless you are actually a lookup or archival node. (#3530)
JamesHinshelwood added a commit that referenced this pull request Apr 4, 2023
…SHARD unless you are actually a lookup or archival node. (#3530) (#3545)

Co-authored-by: Richard Watts <108257153+rrw-zilliqa@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants