Tipset throws IndexOutOfBoundsException if the address book becomes smaller

[rbair23]

Description

While reviewing this method:

    public static Tipset merge(final List<Tipset> tipsets) {
        Objects.requireNonNull(tipsets, "tipsets must not be null");
        if (tipsets.isEmpty()) {
            throw new IllegalArgumentException("Cannot merge an empty list of tipsets");
        }

        final var firstTipset = tipsets.getFirst();
        final int length = firstTipset.tips.length;
        final Tipset newTipset = buildEmptyTipset(firstTipset);

        for (int index = 0; index < length; index++) {
            long max = UNDEFINED;
            for (final Tipset tipSet : tipsets) {
                max = Math.max(max, tipSet.tips[index]);
            }
            newTipset.tips[index] = max;
        }

        return newTipset;
    }

It appears an IOOBE will be thrown on Math.max(max, tipSet.tips[index]); if the address book has become smaller. When merging tipsets, this code takes the first tipset in the list, and uses it for the length. Any subsequent tipset in the merge that is less than this length will not have an index position of "index".

It isn't clear why the first tipset in the list is given preference, or why the new merged tipset should use its length.

Steps to reproduce

A unit test to this effect has not been written, but could be.

Additional context

No response

Hedera network

No response

Version

develop

Operating system

None

[rbair23]

I don't understand why we can use the address book of the first tipset in the list as the address book for the merged result. It seems like the address book to use must be supplied to the method, because it must be the address book of the event for which the merged tipset is being created.

[rbair23]

Is this a more appropriate implementation?

    public static Tipset merge(final AddressBook addressBook, final List<Tipset> tipsets) {
        Objects.requireNonNull(tipsets, "tipsets must not be null");
        if (tipsets.isEmpty()) {
            throw new IllegalArgumentException("Cannot merge an empty list of tipsets");
        }

        final int length = addressBook.getSize();
        final long[] newTips = new long[length];
        for (int index = 0; index < length; index++) {
            long max = UNDEFINED;
            for (final Tipset tipSet : tipsets) {
                max = Math.max(max, tipSet.tips.length >= index ? tipSet.tips[index] : UNDEFINED);
            }
            newTips[index] = max;
        }

        return new Tipset(addressBook, newTips);
    }

[rbair23]

Ah, I see in the documentation:

     * @param tipsets the tipsets to merge, must be non-empty, tipsets must be constructed from the same address book or
     *                else this method has undefined behavior

But if this is the case, then I suppose the datastructures used for tipset must be rebuilt whenever the roster/address book changes. Is that necessary? Or can we suppose address book size changes as described here?

[lpetrovic05]

I think tipset was never modified to work with dynamic address book, this is true for multiple parts of the system