Persist SendPay.groupid as TEXT to cover full u64 range

[ok300]

Restoring a node kept panicking with a ToSqlConversionFailure(TryFromIntError(())) error. Turns out this was caused by how we persist SendPay.groupid.

In cln_grpc this field is defined as uint64 and deserialized as u64:

https://github.com/ElementsProject/lightning/blob/e66653fa1d847e28bd812356a762225a44bfae83/cln-grpc/proto/node.proto#L935

However we map it to an INTEGER in the DB, which actually accepts signed 64 bit numbers. If we try to persist positive integers that use the 64th bit, inserting it into SQL will fail with ToSqlConversionFailure(TryFromIntError(())).

This is exactly what caused restore to fail in my case, as I had several groupids bigger than 2^63.

This PR fixes this by converting SendPay.groupid to String and storing it as TEXT. Since we only use it as a String, we don't have to re-convert it back to u64 when deserializing SendPay from the DB.

[JssDWt]

This PR fixes this by converting SendPay.groupid to String and storing it as TEXT. Since we only use it as a String, we don't have to re-convert it back to u64 when deserializing SendPay from the DB.

If I understand correctly, you had invalid groupids stored in your send_pays table. If we create the new table and insert everything from the old table, I think the groupids migrated from the old table would still be invalid? Potentially we won't identify multiple parts belonging to the same group correctly during sync because of it.

I think it's best to

• drop the send_pays table and recreate it with the new schema
• drop the sync state by removing the sync_state entry from the cache

The sdk will then resync everything.

[ok300]

If I understand correctly, you had invalid groupids stored in your send_pays table.

No, started the CLI with a fresh data directory, so restore only pulled fresh new data.

I think it's best to

• drop the send_pays table and recreate it with the new schema
• drop the sync state by removing the sync_state entry from the cache

The sdk will then resync everything.

Done: 7c16a50

[JssDWt]

A nit, then I'll test

[ok300]

Done.

[dangeross]

utACK

[JssDWt]

Tested this on an existing node, and it works.
Thinking about whether we should also clear the payments table as a precaution for potentially invalid updates. It might not be necessary, but it won't do any harm either. What do you think @ok300?

[roeierez]

Tested this on an existing node, and it works. Thinking about whether we should also clear the payments table as a precaution for potentially invalid updates. It might not be necessary, but it won't do any harm either. What do you think @ok300?

I think it is not necessary because syncing will replace invalid payments.

[roeierez]

LGTM

[JssDWt]

LGTM