pokt-network / poktroll

The official Shannon upgrade implementation of the Pocket Network Protocol implemented using Rollkit.dev
MIT License
15 stars 6 forks source link

[Bug] Ring address ordering makes valid subsets beings rejected #536

Closed red-0ne closed 1 month ago

red-0ne commented 1 month ago

Summary

This PR fixes the RelayRequest signature verification process that fails when delegateeGatewayAddresses ordering differs at on-chain verification time.

Issue

The RelayRequest verification step checks that the ring addresses used to sign the request are contained within the expected ring at proof verification time. For this we use ringSig.Ring().Equal(expectedRing).

The actual implementation of Equal [1] imposes that the request ring pubkeys must be in the same order as the expected one.

Using Equal in the following (requestRingAddrs, expectedRingAddrs) examples would yield undesired results in some cases:

* Equals(AB, ABC) == true  // desired
* Equals(BC, ABC) == false // undesired
* Equals(AC, ABC) == false // undesired

Given our delegation and undelegation use cases with their dynamic aspect, we want the (BC, ABC) and (AC, ABC) cases to yield true.

[1] https://github.com/noot/ring-go/blob/master/ring.go#L23

Type of change

Select one or more:

Testing

Local Testing (only if making code changes)

PR Testing (only if making code changes)

Sanity Checklist

github-actions[bot] commented 1 month ago

The CI will now also run the e2e tests on devnet, which increases the time it takes to complete all CI checks. If you just created a pull request, you might need to push another commit to produce a container image DevNet can utilize to spin up infrastructure. You can use make trigger_ci to push an empty commit.