Vm sdk

[aaronbuchwald]

Why this should be merged

How this works

How this was tested

Need to be documented in RELEASES.md?

[StephenButtolph]

Didn't finish the review - but need to go get dinner + workout

[StephenButtolph]

Suggested change

	f.buffer.Push(key) // Push removes the oldest [K] if we are at the [limit]
	f.buffer.Push(key) // Push calls remove with the oldest key if we are at the limit

[StephenButtolph]

With the recent cache refactor, I think this should be it its own fifo package like how we have lru

[StephenButtolph]

I don't think it is typically recommended to prefix Get for getters in golang

[StephenButtolph]

godoc links don't work for function arguments

Suggested change

	// NewFIFO creates a new First-In-First-Out cache of size [limit].
	// NewFIFO creates a new First-In-First-Out cache of size limit.

[StephenButtolph]

Suggested change

	// remove is used as the callback in [BoundedBuffer]. It is assumed that the
	// [WriteLock] is held when this is accessed.
	// remove is used as the callback in the bounded buffer. It is assumed that the
	// write lock is held when this is called.

[StephenButtolph]

When first reading this, I thought defaultConfig would be returned unless overridden.

But in reality, it is a config storing the default values and each individual value are used unless overridden. Perhaps we should rename the var, or perhaps we should just document this function.

[StephenButtolph]

Suggested change

	func GetConfig[T any](c Config, key string, defaultConfig T) (T, error) {
	func FillConfig[T any](c Configs, configName string, defaultConfig T) (T, error) {

I still don't love the name, but it feels a bit more correct than GetConfig.

We could even change the function to:

func FillConfig[T any](cs Config, name string, c *T) error {
	val, ok := cs[name]
	if !ok {
		return nil
	}
	return json.Unmarshal(val, c)
}

[StephenButtolph]

Generic types are typically a single character in golang. I think it makes the code read more easily here.

Suggested change

	func Map[Input any, Output any](mapF func(Input) Output, sub Subscription[Output]) Subscription[Input] {
	return SubscriptionFunc[Input]{
	NotifyF: func(ctx context.Context, t Input) error {
	return sub.Notify(ctx, mapF(t))
	},
	Closer: sub.Close,
	}
	}
	func Map[I, O any](f func(I) O, sub Subscription[O]) Subscription[I] {
	return SubscriptionFunc[I]{
	NotifyF: func(ctx context.Context, v I) error {
	return sub.Notify(ctx, f(v))
	},
	Closer: sub.Close,
	}
	}

[StephenButtolph]

I waffled a lot on this description. It really isn't the format I'm used to for a Map function... Typically I would think Map would take in a Stream[I] and f(I) O and return a Stream[O]. But instead this takes in an EventHandler[O] and f(I) O and returns an EventHandler[I]...

Perhaps:

Suggested change

	// Map transforms a subscription from an output sub to an input typed sub
	// Map returns a new subscription which transforms an event with f and notifies
	// the original subscription of the result.
	//
	// The original subscription will be closed if the returned subscription is closed.

[StephenButtolph]

Suggested change

	mappedSub := Map(func(eventID string) ids.ID { return ids.FromStringOrPanic(eventID) }, sub)
	mappedSub := Map(ids.FromStringOrPanic, sub)