Persist and reload message counters on the controller

[sagar-apple]

Problem

The controller never persists message counters used for the PASE session but allows persisting the session in storage. This means that each time a connection is stored away and reused later, the counter values are lost.
Resuming these PASE sessions from storage causes the counters to go out of sync and messages get dropped.

I'm seeing this happen every time while trying to provision an M5Stack via the chip-tool cli

Change overview

• Persist the session message counters every time a CHIPDevice is released/destroyed.
• Reload the message counters when a session is reestablished.
• Minor unrelated code cleanup in objc

Testing

How was this tested? (at least one bullet point required)

• If new unit tests are not added, why not?

While I did get some coverage from the Darwin cluster tests, I'm not entirely sure how to unit test this specific change. We're missing some test infrastructure here that prevents me from adding a test easily. I need mock persistent storage + the ability to create and destroy CHIPDevices + connections to ensure they're persisting values correctly.

• If manually tested, what platforms controller and device platforms were manually tested, and how?

Tested ble-pairing via the chip-tool cli on a mac and CHIPTool on iOS with the all-clusters-app on a M5Stack. Verified that while pairing, message counter errors are no longer seen and that as we transition from pairing -> network provisioning the messages coming in are being sent with the right message counter values.

[todo]

no need to base-64 the serialized values AGAIN

connectedhomeip/src/controller/CHIPDevice.cpp

Lines 517 to 524 in 6dce442

	// TODO: no need to base-64 the serialized values AGAIN
	PERSISTENT_KEY_OP(GetDeviceId(), kPairedDeviceKeyPrefix, key,
	mStorageDelegate->SyncSetKeyValue(key, serialized.inner, sizeof(serialized.inner)));
	}
	}
	} // namespace Controller
	} // namespace chip

---

This comment was generated by todo based on a TODO comment in 6dce442 in #7106. cc @sagar-apple.

[turon]

The specification calls for only persisting the global message counters used for group messages. Session counters are only meant to persist for the life of a session, and sessions are to be reestablished from scratch if there is a sync issue or reboot.

[github-actions]

Size increase report for "nrfconnect-example-build" from 53e7847

File	Section	File	VM
chip-lock.elf	rodata	32	28
chip-lock.elf	text	28	28
chip-lock.elf	device_handles	4	4
chip-shell.elf	rodata	24	28
chip-shell.elf	text	28	28
chip-shell.elf	device_handles	-12	-12

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-lock.elf and ./pull_artifact/chip-lock.elf:

sections,vmsize,filesize
.strtab,0,161
.debug_line,0,157
.debug_frame,0,112
.symtab,0,96
.debug_abbrev,0,59
.debug_aranges,0,32
.debug_ranges,0,32
rodata,28,32
text,28,28
.debug_str,0,24
.debug_loc,0,16
device_handles,4,4
.shstrtab,0,-1
.debug_info,0,-1088

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.debug_info,0,239
.strtab,0,161
.symtab,0,96
.debug_frame,0,56
.debug_line,0,52
.debug_abbrev,0,29
rodata,28,24
text,28,28
.debug_str,0,24
.debug_aranges,0,16
.debug_ranges,0,16
.shstrtab,0,-1
device_handles,-12,-12
.debug_loc,0,-16

[github-actions]

Size increase report for "esp32-example-build" from 53e7847

File	Section	File	VM
chip-all-clusters-app.elf	.flash.rodata	24	24
chip-all-clusters-app.elf	.flash.text	16	16

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.debug_info,0,1516
.debug_loc,0,620
.debug_line,0,379
.shstrtab,0,251
.debug_str,0,200
.xt.prop._ZN4chip29GlobalEncryptedMessageCounter10SetCounterEj,0,196
.strtab,0,161
.debug_frame,0,120
.symtab,0,112
.xt.prop._ZN4chip26LocalSessionMessageCounter10SetCounterEj,0,76
.xt.prop._ZN4chip31GlobalUnencryptedMessageCounter10SetCounterEj,0,76
.debug_abbrev,0,56
.debug_aranges,0,40
.xt.lit._ZN4chip9Transport19PeerConnectionStateC5Ev,0,40
.debug_ranges,0,32
.flash.rodata,24,24
.flash.text,16,16
.xt.prop._ZTVN4chip11DeviceLayer37DeviceNetworkProvisioningDelegateImplE,0,1
[Unmapped],0,-24
.xt.prop._ZN4chip9Transport19PeerConnectionStateC5Ev,0,-120

[github-actions]

Size increase report for "gn_qpg6100-example-build" from 53e7847

File	Section	File	VM
chip-qpg6100-lighting-example.out	.text	60	60

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-qpg6100-lighting-example.out.map and ./pull_artifact/chip-qpg6100-lighting-example.out.map:

BLOAT EXECUTION FAILED WITH CODE 1:
bloaty: unknown file type for file './pull_artifact/chip-qpg6100-lighting-example.out.map'

Comparing ./master_artifact/chip-qpg6100-lighting-example.out and ./pull_artifact/chip-qpg6100-lighting-example.out:

sections,vmsize,filesize
.strtab,0,161
.debug_line,0,156
.debug_frame,0,112
.symtab,0,112
.debug_loc,0,110
.text,60,60
.debug_abbrev,0,59
.debug_aranges,0,32
.debug_ranges,0,32
.debug_str,0,25
.shstrtab,0,-1
[Unmapped],0,-60
.debug_info,0,-1094

[andy31415]

should we remove this comment? I do not see a base-64 serialization here. I realize it used to be in moved code ... guessing base64 was removed but comment was left. We can clean it up now.

[sagar-apple]

I think we are still base-64 encoding it twice right? Once because the device supports serializing to a SerializedDevice and then again because the Darwin storage delegate converts everything to base-64

[andy31415]

it was non-obvious to me because 'inner' is not type-defined.

if it would be inner.base64 that would be understandable. However now it is inner and sizeof(inner) so everything sure looks binary to me.

Consider moving inside the serialize.inner reading/writing since this is were we say "this is binary, no need for base64".

[andy31415]

should these comments be part of the struct? I believe that this should be part of the serialize/deserialize methods like "serializes 16-bit and larger values as little endian".