[DRAFT] chore(framework): Remove the Bridge package and all its functionalities #6863
+2,151
−42,635
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 4 Skipped Deployments
|
iota-ci
added
sc-platform
miker83z
reviewed
May 19, 2025
| @@ -1540,24 +1534,6 @@ pub fn generate_genesis_system_object( | |||
| vec![], | |||
| )?; | |||
|
|
|||
| if protocol_config.enable_bridge() { | |||
There was a problem hiding this comment.
Comment for other reviewers:
- we are safe when removing this here because in both Mainnet and Testnet genesis processes
enable_bridgewas false
miker83z
reviewed
May 19, 2025
| @@ -47,7 +47,6 @@ impl From<ProtocolConfigValue> for IotaProtocolConfigValue { | |||
| ProtocolConfigValue::u16(y) => IotaProtocolConfigValue::U16(y), | |||
| ProtocolConfigValue::u32(y) => IotaProtocolConfigValue::U32(y), | |||
| ProtocolConfigValue::u64(x) => IotaProtocolConfigValue::U64(x), | |||
| ProtocolConfigValue::bool(z) => IotaProtocolConfigValue::Bool(z), | |||
miker83z
reviewed
May 19, 2025
| @@ -161,10 +161,6 @@ struct FeatureFlags { | |||
| #[serde(skip_serializing_if = "is_false")] | |||
| enable_jwk_consensus_updates: bool, | |||
|
|
|||
| // Enable bridge protocol | |||
| #[serde(skip_serializing_if = "is_false")] | |||
| bridge: bool, | |||
There was a problem hiding this comment.
@muXxer @alexsporn is it safe to be completely removed even though it is not used anymore?
There was a problem hiding this comment.
AFAIK we can remove protocol feature flags without problems if the snapshot files don't change. And the bridge was never enabled, so the snapshot files should be the same.
miker83z
reviewed
May 19, 2025
| @@ -632,16 +632,6 @@ impl From<crate::transaction::EndOfEpochTransactionKind> for EndOfEpochTransacti | |||
| .authenticator_obj_initial_shared_version | |||
| .value(), | |||
| }), | |||
| crate::transaction::EndOfEpochTransactionKind::BridgeStateCreate(chain_identifier) => { | |||
There was a problem hiding this comment.
TODO: update the sdk2 repo once this PR is merged
Dkwcs
requested review from
luca-moser,
lzpap,
alexsporn and
a team
as code owners
miker83z
reviewed
May 19, 2025
Comment on lines
-717
to
-725
| EndOfEpochTransactionKind::BridgeStateCreate(chain_id) => { | ||
| assert!(protocol_config.enable_bridge()); | ||
| builder = setup_bridge_create(builder, chain_id) | ||
| } | ||
| EndOfEpochTransactionKind::BridgeCommitteeInit(bridge_shared_version) => { | ||
| assert!(protocol_config.enable_bridge()); | ||
| assert!(protocol_config.should_try_to_finalize_bridge_committee()); | ||
| builder = setup_bridge_committee_update(builder, bridge_shared_version) | ||
| } |
There was a problem hiding this comment.
Comment for other reviewers:
- should be safe to completely remove this without adding a new cut because this transaction Kind was never executed in both Mainnet and Testnet.
This reverts commit 161fa52.
…e-6847-remove-bridge
muXxer
reviewed
May 20, 2025
| @@ -161,10 +161,6 @@ struct FeatureFlags { | |||
| #[serde(skip_serializing_if = "is_false")] | |||
| enable_jwk_consensus_updates: bool, | |||
|
|
|||
| // Enable bridge protocol | |||
| #[serde(skip_serializing_if = "is_false")] | |||
| bridge: bool, | |||
There was a problem hiding this comment.
AFAIK we can remove protocol feature flags without problems if the snapshot files don't change. And the bridge was never enabled, so the snapshot files should be the same.
| /// Whether to try to form bridge committee | ||
| // Note: this is not a feature flag because we want to distinguish between | ||
| // `None` and `Some(false)`, as committee was already finalized on Testnet. | ||
| bridge_should_try_to_finalize_committee: Option<bool>, |
There was a problem hiding this comment.
I'm not 100% sure we can remove this one.... the snapshot files seem to change. Is this allowed?
This reverts commit 30bf4a2.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[run-ci]
Description of change
Please write a summary of your changes and why you made them.
Links to any relevant issues
fixes #6847
Type of change
Choose a type of change, and delete any options that are not relevant.
How the change has been tested
Describe the tests that you ran to verify your changes.
Make sure to provide instructions for the maintainer as well as any relevant configurations.
Infrastructure QA (only required for crates that are maintained by @iotaledger/infrastructure)
Change checklist
Tick the boxes that are relevant to your changes, and delete any items that are not.
Release Notes