[EasyUnlock] Add beacon_seeds to RemoteDevice.

components/cryptauth/remote_device.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef COMPONENTS_CRYPTAUTH_REMOTE_DEVICE_H_
 #define COMPONENTS_CRYPTAUTH_REMOTE_DEVICE_H_
 
 #include <string>
 #include <vector>
 
+#include "components/cryptauth/proto/cryptauth_api.pb.h"
+
 namespace cryptauth {
 
 struct RemoteDevice {

[Kyle Horimoto, 2017/05/04 03:52:51]

Please store a boolean (maybe call it has_fetched_beacon_seeds?) so that you can
differentiate between devices with no valid beacon seeds and devices which have
not yet been fetched.

[Tim Song, 2017/05/06 22:53:16]

I think this is problematic because this struct is inherently mutable. Anyone
can change the beacon_seeds but forget to change this has_fetched_beacon_seeds
field. 

I think it's safest just not to compare the beacon_seeds. IMO, it is undefined
behaviour to have a device with the same public keys but different BeaconSeeds
in any case.

[Ryan Hansberry, 2017/05/09 02:32:19]

Re: "Anyone can change the beacon_seeds but forget to change this
has_fetched_beacon_seeds field" -- You can make beacon_seeds and
has_fetched_beacon_seeds private fields which are simultaneously changed by a
public setter method, so I don't agree that that's a real problem.

I agree with Kyle that something like a has_fetched_beacon_seeds field should
exist.

[Tim Song, 2017/05/10 21:58:08]

The biggest problem still is that it's not really useful or particularly defined
to check the beacon seeds for equality.

You don't need it for magic tether, and we certainly don't need it either.
Moreover, there are questions of whether order or BeaconSeeds should matter.

Given that no caller even needs to compare the beacon seeds this way, it's seems
like this is just extraneous code with a chance of introducing undesired
behaviour.

In fact, I'm thinking it might even be better to remove this operator==
altogether, and simply use the public key for equality.

[Ryan Hansberry, 2017/05/15 15:43:34]

I'm inclined to agree that operator== isn't really needed, but it still seems
useful to me to be able to tell if a RemoteDevice has no BeaconSeeds because it
hasn't synced yet or truly has no BeaconSeeds.

  public:
   std::string user_id;
   std::string name;
   std::string public_key;
   std::string bluetooth_address;
   std::string persistent_symmetric_key;
   std::string sign_in_challenge;
 
+  // Note: To save space, the BeaconSeeds may not necessarily be included in
+  // this object.
+  std::vector<BeaconSeed> beacon_seeds;
+
   RemoteDevice();
   RemoteDevice(const std::string& user_id,
                const std::string& name,
                const std::string& public_key,
                const std::string& bluetooth_address,
                const std::string& persistent_symmetric_key,
                std::string sign_in_challenge);
   RemoteDevice(const RemoteDevice& other);
   ~RemoteDevice();
 
@@ skipping 13 matching lines @@
 
   // Static method for truncated device ID for logs.
   static std::string TruncateDeviceIdForLogs(const std::string& full_id);
 };
 
 typedef std::vector<RemoteDevice> RemoteDeviceList;
 
 }  // namespace cryptauth
 
 #endif  // COMPONENTS_CRYPTAUTH_REMOTE_DEVICE_H_