Cache and resend, rather than recreate, builder API registrations #4040
Conversation
| continue | ||
| if validator.externalBuilderRegistration.isNone: | ||
| let validatorRegistration = | ||
| await node.getValidatorRegistration(validator, epoch) |
There was a problem hiding this comment.
if someone adds a validator via rest api, I'm guessing this await can get interleaved (cc @zah?)
There was a problem hiding this comment.
In general, there are lots of the usual kinds approaches people use when modifying sequences in place, but for this purpose, it mostly matters whether table mvalues iteration works with that. It's fine if some new validator is missed this epoch around in that scenario, and it's even fine to just abort the table scan in a given epoch if there's any issues of this sort. I'd prefer either, ideally, to adding more internal buffering to this function of information on all attached validators, but that could, ultimately, solve this.
There was a problem hiding this comment.
The problem here is that the contents of the validators table may change while you are iterating over it. This would be detected as a Defect by the standard library.
The standard work-around is to copy the list of validators prior to starting the iteration by iterating over dup(node.attachedValidators[].validators.values).
There was a problem hiding this comment.
cf539c9 works around this. Rather than copy the values in their entirety, which would be a better approach for a read-only application, here it copies the keys, because it's more obviously symmetrical that the access mechanism that works for reading also works for writing.
#3998