Fix runtime ID, check amount when minting collectibles, rebase GetInfo RPC fixes commit #589
Conversation
|
Would be good to have an itest for the runtime ID change; something like starting 100 |
| @@ -275,7 +276,13 @@ func genServerConfig(cfg *Config, cfgLogger btclog.Logger, | |||
| SyncBatchSize: defaultUniverseSyncBatchSize, | |||
| }) | |||
|
|
|||
| runtimeID := prand.Int63() // nolint:gosec | |||
| var runtimeIDBytes [8]byte | |||
There was a problem hiding this comment.
Any reason to not increase to 32 bytes so we don't have to think about it?
There was a problem hiding this comment.
Could do both but p sure root cause was the RNG.
There was a problem hiding this comment.
Any reason to not increase to 32 bytes so we don't have to think about it?
Yeah, just to make things non-breaking on the RPC interface.
I think 8 bytes should be waaaay enough for this. I'm pretty sure this was a combination of weak RNG+docker+macOS that lead to duplicates. I tried to reproduce this on my machine with a dummy docker setup and wasn't able to.
Using the better RNG should definitely make this almost impossible to hit.
Unfortunately neither a unit nor an integration test can help here, since as long as things are in the same process, we can never get a collision as even the weak RNG is a package-level variable. So if code from the same process request randomness, it will be different for each call as the calls are serialized. This can only be reproduced by spawning multiple processes within a very short time (which isn't super easy to do with our current setup).
I'm voting to just leave things the way they are and rely on the CPRNG being much better for this.
Unit test here (forces us to abstract it a bit more) would prob be good enough here. Or we increase increase to 32 bytes and call it a day. |
Fixes #573 by using the cryptographic pseudo number generator for generating the runtime ID. Apparently when starting two nodes at the very same time the math/rand library can generate the same ID on different processes.
Fixes #576 by making sure that the amount for collectible assets is always set at 1.
… and block hash Added additional fields to the GetInfo RPC response to provide richer details about the blockchain and the LND node. This includes the LND node's public key (identity), its alias, the current block height, and the latest block's hash.
Fixes #576.
Fixes #573.
Replaces #554.
Fixes multiple small issues.