Refined Storage

Refined Storage

115M Downloads

Network Transmitter leads to security breaking (defaults to never allow anything)

Closed this issue ยท 7 comments

commented

Describe the bug

https://discord.com/channels/342942776494653441/844238333063987220/1361500580480286810

I didn't get this myself, just making a report on behalf of flames fireclaw and sand turtle

effects: the network thinks nobody has permissions to do anything with it, regardless of op or security status
possibly related to #943

How can we reproduce this bug or crash?

place a transmitter and receiver
controller on both sides
break the one on the receiver side
https://imgur.com/RpVzJYV

What Minecraft version is this happening on?

Minecraft 1.21.1

What NeoForge or Fabric version is this happening on?

21.1.142

What Refined Storage version is this happening on?

beta 2

Relevant log output

commented

This doesn't always result in bugged permission, but it reliably disconnects the receiver from the transmitting network. And fixing that should fix the permissions as well.

commented

Also having this issue.

commented

Just experienced this as well

commented

Just had this happen as well, is there any workaround or fix for this for when it happens?

commented

Restart the server, stop using network transmitters

commented

Special cases (maybe?)

commented

There was a bug in the merge algorithm for nodes that are connected via a one-way connection.

In following network topology:
Network A [wireless one way->] Network A

If you cause a merge, like so:
Network A [wireless one way->] Network A [<->] Network B

If Network B is the pivot of the merge, the network of nodes in Network A (on the right side of the one-way connection) will be replaced with Network B.
You end up with following topology after merge:
Network A [wireless one way->] Network B

This is what causes the nodes of the network receiver to disconnected, since the nodes in Network B no longer have a power source provided in Network A.

So far, this is completely expected and normal. The right side of the one-way connection is no longer able to trace back to the original Network A, since it's a one-way connection.

What should happen at that point is the reconnection logic of the network transmitter in Network A reconnecting and reuniting with Network B:

    @Override
    public void doWork() {
        super.doWork();
        final boolean receiverFound = isReceiverFoundInNetwork(mainNetworkNode.getNetwork(), receiverKey);
        if (!receiverFound && networkRebuildRetryRateLimiter.tryAcquire()) {
            tryReconnectingWithReceiver();
        }
    }

However, this wasn't happening.
Why?

There was an oversight in the merge algorithm, where it didn't think about one-way connections.
When it was merging, it was just replacing the network of all found nodes with the network of the pivot in merge:

    @Nullable
    private Network mergeNetworkOfNode(final Network newNetwork,
                                       final NetworkNodeContainer entry,
                                       final NetworkNode entryNode) {
        final Network oldNetwork = entryNode.getNetwork();
        entryNode.setNetwork(newNetwork);
        newNetwork.addContainer(entry);
        return oldNetwork;
    }

This works if the old network is no longer relevant (99% of cases), since it's a complete merge of all the found nodes.
However, if the merge origin / pivot is on the right side of the one-way connection:

  • The nodes on the left side of the one-way connection will be untouched (as they are not found to begin with)
  • The nodes on the right side of the one-way connection will "linger" in the left side of the one-way connection

The fix for this was to do the following:

    @Nullable
    private Network mergeNetworkOfNode(final Network newNetwork,
                                       final NetworkNodeContainer entry,
                                       final NetworkNode entryNode) {
        final Network oldNetwork = entryNode.getNetwork();
        entryNode.setNetwork(newNetwork);
        newNetwork.addContainer(entry);
+       if (oldNetwork != null) {
+          oldNetwork.removeContainer(entry);
+       }
        return oldNetwork;
    }

Special thanks to following comments that gave me more insight:

Next steps:

  • I want to understand why this fix also fixes the later exception java.lang.IllegalStateException: Network of resulting removed node (com.refinedmods.refinedstorage.api.network.impl.node.SimpleNetworkNode) cannot be empty
  • My fix needs tests

I've created some debugging tools to help investigate this issue, mainly being a highlighter that assigns a unique color for a network, and a debug stick that dumps the network graph:

Image