[script] Synchronized integer

12-06-2010, 07:43 PM

This is an example script showing how to safely work with values that may be updated using local values from individual players.

Explanation
The goal is to allow a player who knows a value unknown to the other players to communicate that value. For example, maybe one player is a bot that saves a leaderboard and needs to give that information to the others.

The system works by syncing game cache values. A player wanting to send a value temporarily stores it in game cache and syncs it. The 'sync' will then travel to the host as a game action (exactly like unit orders do), the host will include it in the next periodic tick packet, and all players will receive the tick and apply the synced value at the same game time (tick packets work that way). Note that this means changing the value does not happen instantly. The sending player has to be careful to use the old value until the new synced value arrives (the system takes care of it).

One caveat to all of this is that there is no authentication on game cache. A hacked map or game client can easily send fake values and cause problems. Keep this in mind! Do NOT store crucial values, like a to-be-applied damage bonus, in a synced struct!

Code
There may be stupid syntax errors because I don't have the editor handy on this laptop.

JASS:

///////////////////////////////////////////////////////////////////
/// Sync Types v1.0
/// Dec 9, 2010
/// Strilanc
///////////////////////////////////////////////////////////////////
/// Implements types with desync-safe values. SyncInt, SyncReal and SyncBoolean.
///
/// Works by syncing new values to game cache, sacrificing instantaneous writes for
/// desync-safety. (Syncs travel to the host and back before they take effect.)
///
/// Note: Game cache is not authenticated. Malicious clients can inject fake values.
/// Avoid storing important non-authenticated values in a SyncType.
///////////////////////////////////////////////////////////////////
library SyncTypes initializer init
    globals
        private gamecache gc
        private constant string FILENAME = "SyncTypes.gc"
        private integer nextId = 0
    endglobals
 
    //! textmacro DefineSyncType takes name, type, Type
        struct $name$
            private string key
            
            ///Modifies the locally stored value.
            private method store takes $type$ value returns nothing
                call Store$Type$(gc, "$name$", .key, value)
            endmethod
            ///Sends a game action which eventually causes all players to update to the sent local value at the same game time.
            private method sync takes nothing returns nothing
                call SyncStored$Type$(gc, "$name$", .key)
            endmethod
                
            ///Returns the currently assigned value.
            public method get takes nothing returns $type$
                return GetStored$Type$(gc, "$name$", .key)
            endmethod

            ///Starts the process of assigning a value potentially known by only one player to the synced value.
            ///The assigned value will update after the sync action reaches the host and bounces back to all players.
            public method syncTo takes $type$ newValue returns nothing
                local $type$ curValue = .get()
                call .store(newValue)
                call .sync()
                call .store(curValue)
            endmethod

            public static method create takes $type$ initialValue returns thistype
                local thistype this = thistype.allocate()
                set .key = I2S(nextId)
                set nextId = nextId + 1
                call .store(initialValue)
                return this
            endmethod
            private method onDestroy takes nothing returns nothing
                call FlushStored$Type$(gc, "$name$", .key)
                call .sync() //will flush stale values from any unfinished syncTo actions, preventing leaks
            endmethod
        endstruct
    //! endtextmacro

    //! runtextmacro DefineSyncType("SyncInt", "integer", "Integer")
    //! runtextmacro DefineSyncType("SyncReal", "real", "Real")
    //! runtextmacro DefineSyncType("SyncBool", "boolean", "Boolean")
    //note: Unit not included because it requires X and Y side-information
    //note: String not included because its sync action is bugged and does nothing

    private function init takes nothing returns nothing
        call FlushGameCache(InitGameCache(FILENAME))
        set gc = InitGameCache(FILENAME)
    endfunction
endlibrary

12-06-2010, 08:56 PM	#2
Anitarf	An explanation at the start of the library regarding how and why it works would be nice (it seems you store the new value to gamecache, then sync it, then store the old value again. Is this some sort of trick?) as well as a giving us an example of how to use this.

12-06-2010, 10:05 PM

Strilanc

Quote:

Originally Posted by Anitarf

An explanation at the start of the library regarding how and why it works would be nice (it seems you store the new value to gamecache, then sync it, then store the old value again. Is this some sort of trick?) as well as a giving us an example of how to use this.

The reason the old value is restored is because the other players still also have the old value. The new value is only kept long enough to make it the value of the sync action. Once the sync travels to the host and back everyone gets the new value at the same game time.

12-07-2010, 12:09 AM	#4
Anitarf	I see. What about locking, what role does that play? It seems from your description that syncTo already functions without error on its own.

12-07-2010, 11:08 AM

Strilanc

Quote:

Originally Posted by Anitarf

I see. What about locking, what role does that play? It seems from your description that syncTo already functions without error on its own.

Locking prevents an easy to make error where a running sync overwrites a normal set. It also prevents some malicious modification of the value, because game cache syncs aren't authenticated in any sense.

I updated the main post.

12-07-2010, 12:14 PM

Anitarf

Quote:

Originally Posted by Strilanc

What would be an example of a player setting a value of an integer that is normally being synced from only one player? I can't quite imagine a situation where I'd want to do anything other with the syncInt besides having it periodically/occasionally synced with a local value.

On the topic of malicious syncing (I guess you mean by a third party program running alongside WC3, or do you mean a modified map that still registers as the same map, but has additional syncing code?) you could probably get more safety (at least in the case of external programs) by obfuscating the key. Instead of simply doing set .key = I2S(this), do some encryption using a random number chosen at the start of the map and some hashing or whatever. Currently, the locking itself seems easy to bypass by periodically sending sync requests, eventually one of them will land in the brief interval during which the user has syncInt unlocked to sync a value.

12-07-2010, 01:22 PM

Strilanc

Quote:

Originally Posted by Anitarf

A modified map with additional syncs injected, used alongside a hacked wc3 which doesn't enforce the map checksum.

Obsfucating the key is pointless in that case. The attacker has all the same state.

Having untrusted values that don't change after some point is better than total hacker freedom. After the sync phase you can lock the value (unfortunately there is no event for having received a synced value and an attacker can mess with any side syncs to try to allow events).

I'm having a bit of trouble coming up with a non-security reason to lock the values. Some case where a value can source from common knowledge or solo knowledge I guess. Maybe a loading system supporting both save codes and hacky stuff for hard disk access? Enter a save code and stats are locked to the interpreted values instead of anything coming from the disk.

Making game cache secure is a bit of a problem, really.

12-07-2010, 02:05 PM

Anitarf

Quote:

Originally Posted by Strilanc

A modified map with additional syncs injected, used alongside a hacked wc3 which doesn't enforce the map checksum.

Obsfucating the key is pointless in that case. The attacker has all the same state.

Indeed. However, in such a case, locking doesn't help much either, the edited map can simply have all the locking calls removed.

Quote:

Having untrusted values that don't change after some point is better than total hacker freedom. After the sync phase you can lock the value (unfortunately there is no event for having received a synced value and an attacker can mess with any side syncs to try to allow events).

I'm having a bit of trouble coming up with a non-security reason to lock the values. Some case where a value can source from common knowledge or solo knowledge I guess. Maybe a loading system supporting both save codes and hacky stuff for hard disk access? Enter a save code and stats are locked to the interpreted values instead of anything coming from the disk.

Seems to me that in the cases you describe, users could easily lock the values themselves by reading them from the SyncInt once and storing them to a global that they use from that point on.

Also, would it make sense to add a SyncReal struct? It seems like it could be useful for syncing things like camera coordinates (although converting reals to integers and back isn't such a huge problem).

Quote:

Making game cache secure is a bit of a problem, really.

Yeah, but it's no less secure than typical save/load codes, we just have to accept it and take other measures to detect potential abuses (such as checking if the loaded hero has any discrepancies in his stats that couldn't be achieved through regular play).

12-07-2010, 02:31 PM

Strilanc

Quote:

Originally Posted by Anitarf

Indeed. However, in such a case, locking doesn't help much either, the edited map can simply have all the locking calls removed.

Seems to me that in the cases you describe, users could easily lock the values themselves by reading them from the SyncInt once and storing them to a global that they use from that point on.

Also, would it make sense to add a SyncReal struct? It seems like it could be useful for syncing things like camera coordinates (although converting reals to integers and back isn't such a huge problem).

Removing the locking calls doesn't remove them in the other players' maps. Any synced value would be ignored by the normal players, leading to a desync if the attacker did not ignore the new value.

It does make a lot of sense to read from the SyncInt and write to a global (even just for efficiency reasons). That's certainly one way to use the struct (as a communication primitive instead of a storage primitive).

Even if the locking code is ultimately not worthwhile for functionality, it does make people ask why it's there. That leads to understanding what is and isn't guaranteed by the system, which is certainly not obvious.

Maybe it makes more sense to do something more like prepareToReceive/Send/lockWhenReceived.

Writing SyncReal, SyncUnit and SyncBoolean is trivial given SyncInt. I might rewrite it to use a macro for all four. (Unfortunately, string syncing does not work. Even the game action for it doesn't exist.)

12-07-2010, 03:47 PM

#10

Anitarf

Quote:

Originally Posted by Strilanc

Removing the locking calls doesn't remove them in the other players' maps. Any synced value would be ignored by the normal players, leading to a desync if the attacker did not ignore the new value.

That's true, I didn't properly consider it.

Quote:

It does make a lot of sense to read from the SyncInt and write to a global (even just for efficiency reasons). That's certainly one way to use the struct (as a communication primitive instead of a storage primitive).

That was my first instinct: to think of this as an integer synchronizer, not a synchronised integer. Perhaps something like this:

JASS:

struct synchronizer
    method isSynced takes nothing returns boolean
        return GetStoredBolean(gc, I2S(this), "synced")
    endmethod

    method syncInt takes integer localValue returns nothing
        call StoreInteger(gc, I2S(this), "int", localValue)
        call StoreBoolean(gc, I2S(this), "synced", true)
        call SyncInteger(gc, I2S(this), "int")
        call SyncBoolean(gc, I2S(this), "synced")
        call StoreBoolean(gc, I2S(this), "synced", false)
    endmethod
    method getSyncedInt takes nothing returns integer
        debug if not .isSynced then
            debug //give error
        debug endif
        return GetStoredInteger(gc, I2S(this), "int")
    endmethod
    //repeat for other useful types (probably only real)

    static method create takes nothing returns synchronizer
        local synchronizer this = synchronizer.allocate()
        call StoreBoolean(gc, I2S(this), "synced", false)
        return this
    endmethod
    method onDestroy takes nothing returns nothing
        call FlushStoredMission(gc, I2S(this))
    endmethod
endstruct

Quote:

Even if the locking code is ultimately not worthwhile for functionality, it does make people ask why it's there. That leads to understanding what is and isn't guaranteed by the system, which is certainly not obvious.

The thing is, such functionality still needs documentation to explain it. If it's only reason is to explain, then just putting a good explanation in the documentation should do the job well enough.

12-07-2010, 04:07 PM

#11

Strilanc

It's a bit more complicated than that. Once you get into the communication stuff you quickly want:

- The source of the message.
- The ability to send large messages (strings).
- An event to know a message has arrived.

Each of those is extremely useful, but require more and more supporting machinery.

Thinking more about it, I think I will remove the locking, leaving only the super-basic synced storage mechanism.

*edit* done

12-07-2010, 04:28 PM	#12
Strilanc	Did the text macro thing so allow bools and reals.

12-07-2010, 04:59 PM

#13

Anitarf

While this is not important for periodically synced values (where outside code simply grabs the last synced value), the "synced" boolean from my version is very much needed for one-time syncs (where outside code must wait for the first and only sync to complete). I suppose that functionality could be built on top of the current setup, so I won't insist you add it here.

The thing I will ask you to consider is encapsulating all types in one struct, again like I did in my example. So, one synchroniser struct that has sync*, get* and set* methods for integer, real, boolean. Again, I won't insist on this, but if you think the current design is better please post your reasoning.

I think the initialValue argument for the .create method is superfluous, though, considering you have the .store (should it be named .set instead?) method. Also, you should flush gamecache entries in the onDestroy method. It's only proper to do so, even if there's no danger of leaking because the keys get reused in new structs.

12-07-2010, 05:16 PM

#14

Strilanc

Splitting the types makes it more obvious what you're using when you use the synced values. A SyncInt is clearer than a SyncSomething. You can do the one-time value thing by using a SyncInt and SyncBool together.

The initial value thing is a preference. It also prevents some of the potential bugs involving reused structs being affected by syncs running during a destroy.

Flushing the value on destroy makes sense. Might be canceled by a running sync, but not usually.

12-07-2010, 05:32 PM

#15

Anitarf

Yes, initial value is a preference, my point was, if a user has that preference, he can always call .store after creating the struct. It doesn't completely prevent bugs anyway since it is possible for a struct index to be reused the moment it is destroyed, in which case the .create runs at the same game time as .onDestroy did. The only way to properly account for still-running syncs would be to add a .release method to the struct that only calls .destroy after a short delay.