Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Signature Aggregator ACP-118 updates #422

Merged
merged 12 commits into from
Aug 21, 2024
Merged
8 changes: 4 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ module github.com/ava-labs/awm-relayer
go 1.21.12

require (
github.com/ava-labs/avalanchego v1.11.11-0.20240729205337-a0f7e422bb84
github.com/ava-labs/subnet-evm v0.6.8-acp-118-handlers
github.com/ava-labs/teleporter v1.0.5-0.20240807150146-1c7df1ab5033
github.com/ava-labs/avalanchego v1.11.11-0.20240807214427-376688b4912b
github.com/ava-labs/subnet-evm v0.6.9-0.20240809021034-3ceec5c96a5f
github.com/ava-labs/teleporter v1.0.5-0.20240809021837-ae1f422baa84
github.com/aws/aws-sdk-go-v2 v1.30.3
github.com/aws/aws-sdk-go-v2/config v1.27.9
github.com/aws/aws-sdk-go-v2/service/kms v1.35.3
Expand All @@ -25,7 +25,7 @@ require (
)

require (
github.com/ava-labs/coreth v0.13.7 // indirect
github.com/ava-labs/coreth v0.13.8-0.20240807212152-f7acfafec094 // indirect
github.com/aws/aws-sdk-go-v2/credentials v1.17.9 // indirect
github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.0 // indirect
github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.15 // indirect
Expand Down
16 changes: 8 additions & 8 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,14 @@ github.com/alexliesenfeld/health v0.8.0/go.mod h1:TfNP0f+9WQVWMQRzvMUjlws4ceXKEL
github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156 h1:eMwmnE/GDgah4HI848JfFxHt+iPb26b4zyfspmqY0/8=
github.com/allegro/bigcache v1.2.1-0.20190218064605-e24eb225f156/go.mod h1:Cb/ax3seSYIx7SuZdm2G2xzfwmv3TPSk2ucNfQESPXM=
github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
github.com/ava-labs/avalanchego v1.11.11-0.20240729205337-a0f7e422bb84 h1:AmPZLlnVREbJ/viK/hDTIVn1bqX8QTB2CFtrBxHwnsw=
github.com/ava-labs/avalanchego v1.11.11-0.20240729205337-a0f7e422bb84/go.mod h1:POgZPryqe80OeHCDNrXrPOKoFre736iFuMgmUBeKaLc=
github.com/ava-labs/coreth v0.13.7 h1:k8T9u/ROifl8f7oXjHRc1KvSISRl9txvy7gGVmHEz6g=
github.com/ava-labs/coreth v0.13.7/go.mod h1:tXDujonxXFOF6oK5HS2EmgtSXJK3Gy6RpZxb5WzR9rM=
github.com/ava-labs/subnet-evm v0.6.8-acp-118-handlers h1:2UVHP9TDh3eS/CuD/XzMy2+3ruu/U2ZUny1eqn4bNB4=
github.com/ava-labs/subnet-evm v0.6.8-acp-118-handlers/go.mod h1:BryiYmD6HWmugkSUpDdj/KyMi7ou5pJzPRNPMbMgpbA=
github.com/ava-labs/teleporter v1.0.5-0.20240807150146-1c7df1ab5033 h1:mqtcLphkixDnYY5h8eQlj058KiVpRJaQgGvuUZcAcXo=
github.com/ava-labs/teleporter v1.0.5-0.20240807150146-1c7df1ab5033/go.mod h1:xDSsLZTLQ68ZXs9tb00VNd4fODqHTs6AD3MX6cyQ00Y=
github.com/ava-labs/avalanchego v1.11.11-0.20240807214427-376688b4912b h1:RVamx4MybSg5cvMTAaLKjJg2vPTaBoXiI3NAk0xfKAs=
github.com/ava-labs/avalanchego v1.11.11-0.20240807214427-376688b4912b/go.mod h1:Sq26Wg2Yz0k0EzueGMamRzw4apv6wmNi7vPjxX4nNSk=
github.com/ava-labs/coreth v0.13.8-0.20240807212152-f7acfafec094 h1:e7TvIKDGzsMsKU3Y4HreRcK7f16EcEPUO04UwSCFtng=
github.com/ava-labs/coreth v0.13.8-0.20240807212152-f7acfafec094/go.mod h1:Ouul9dJouniUIJVX1gDqx8CrHyGvmwZkK28mrgKb/4I=
github.com/ava-labs/subnet-evm v0.6.9-0.20240809021034-3ceec5c96a5f h1:moqjaNZ1RSXL2/yYUXvwSEpIWjmXcWxTORMBHWuwrtE=
github.com/ava-labs/subnet-evm v0.6.9-0.20240809021034-3ceec5c96a5f/go.mod h1:DM/HccJ8PghYk7fA3Cq76OOoVprOtyITICkjwuzEPfg=
github.com/ava-labs/teleporter v1.0.5-0.20240809021837-ae1f422baa84 h1:rZZZ1O5vDc+qSkBzA6SefJJQi8azip0KNAPv9irOW8U=
github.com/ava-labs/teleporter v1.0.5-0.20240809021837-ae1f422baa84/go.mod h1:J9F+AiUImkNaKIqszwTp+eUhYJsMq7Pcdit10jn0IzQ=
github.com/aws/aws-sdk-go-v2 v1.30.3 h1:jUeBtG0Ih+ZIFH0F4UkmL9w3cSpaMv9tYYDbzILP8dY=
github.com/aws/aws-sdk-go-v2 v1.30.3/go.mod h1:nIQjQVp5sfpQcTc9mPSr1B0PaWK5ByX9MOoDadSN4lc=
github.com/aws/aws-sdk-go-v2/config v1.27.9 h1:gRx/NwpNEFSk+yQlgmk1bmxxvQ5TyJ76CWXs9XScTqg=
Expand Down
3 changes: 3 additions & 0 deletions peers/external_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ func (h *RelayerExternalHandler) HandleInbound(_ context.Context, inboundMessage
zap.Stringer("from", inboundMessage.NodeID()),
)
if inboundMessage.Op() == message.AppResponseOp || inboundMessage.Op() == message.AppErrorOp {
if inboundMessage.Op() == message.AppErrorOp {
h.log.Debug("Received AppError message", zap.Stringer("message", inboundMessage.Message()))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is messy checking but I used this to see error codes received from validator nodes which we don't log anywhere right now

}
h.registerAppResponse(inboundMessage)
} else {
h.log.Debug("Ignoring message", zap.Stringer("op", inboundMessage.Op()))
Expand Down
4 changes: 2 additions & 2 deletions scripts/versions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ export GO_VERSION=${GO_VERSION:-$(getDepVersion go)}
# Don't export them as they're used in the context of other calls
# TODO: undo this hack once go.mod is referring to a tag rather than a commit
#AVALANCHEGO_VERSION=${AVALANCHEGO_VERSION:-$(getDepVersion github.com/ava-labs/avalanchego)}
AVALANCHEGO_VERSION=${AVALANCHEGO_VERSION:-a0f7e422bb8497186ebff434f12c4b957cec8d49}
AVALANCHEGO_VERSION=${AVALANCHEGO_VERSION:-376688b4912bf1b3c988167cfabf4ebedc27dc16}
GINKGO_VERSION=${GINKGO_VERSION:-$(getDepVersion github.com/onsi/ginkgo/v2)}

# TODO: undo this hack once go.mod is referring to a tag rather than a commit
#SUBNET_EVM_VERSION=${SUBNET_EVM_VERSION:-$(getDepVersion github.com/ava-labs/subnet-evm)}
SUBNET_EVM_VERSION=${SUBNET_EVM_VERSION:-v0.6.8-acp-118-handlers}
SUBNET_EVM_VERSION=${SUBNET_EVM_VERSION:-3ceec5c96a5f83b09d06767fae83e89f79c5c2c6}

# Set golangci-lint version
GOLANGCI_LINT_VERSION=${GOLANGCI_LINT_VERSION:-'v1.55'}
16 changes: 10 additions & 6 deletions signature-aggregator/aggregator/aggregator.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (

"github.com/ava-labs/avalanchego/ids"
"github.com/ava-labs/avalanchego/message"
networkP2P "github.com/ava-labs/avalanchego/network/p2p"
"github.com/ava-labs/avalanchego/proto/pb/p2p"
"github.com/ava-labs/avalanchego/proto/pb/sdk"
"github.com/ava-labs/avalanchego/subnets"
Expand All @@ -24,7 +25,6 @@ import (
avalancheWarp "github.com/ava-labs/avalanchego/vms/platformvm/warp"
"github.com/ava-labs/awm-relayer/peers"
"github.com/ava-labs/awm-relayer/utils"
msg "github.com/ava-labs/subnet-evm/plugin/evm/message"
"go.uber.org/zap"
"google.golang.org/protobuf/proto"
)
Expand Down Expand Up @@ -120,9 +120,11 @@ func (s *SignatureAggregator) CreateSignedMessage(
return nil, errNotEnoughConnectedStake
}

reqBytes, err := proto.Marshal(
reqBytes := networkP2P.ProtocolPrefix(networkP2P.SignatureRequestHandlerID)
messageBytes, err := proto.Marshal(
&sdk.SignatureRequest{Message: unsignedMessage.Bytes()},
)
reqBytes = append(reqBytes, messageBytes...)
if err != nil {
msg := "Failed to marshal request bytes"
s.logger.Error(
Expand Down Expand Up @@ -426,8 +428,9 @@ func (s *SignatureAggregator) isValidSignatureResponse(
return blsSignatureBuf{}, false
}

var sigResponse msg.SignatureResponse
if _, err := msg.Codec.Unmarshal(appResponse.AppBytes, &sigResponse); err != nil {
sigResponse := sdk.SignatureResponse{}
err := proto.Unmarshal(appResponse.AppBytes, &sigResponse)
if err != nil {
s.logger.Error(
"Error unmarshaling signature response",
zap.Error(err),
Expand Down Expand Up @@ -459,8 +462,9 @@ func (s *SignatureAggregator) isValidSignatureResponse(
)
return blsSignatureBuf{}, false
}

return signature, true
blsSig := blsSignatureBuf{}
copy(blsSig[:], signature[:])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we have a check higher up that the length of signature is the expected length?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly, we do call bls.SignatureFromBytes and bls.Verify on it's result so presumably those wouldn't work if it wasn't correct lenght but an explicit check wouldn't hurt

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 811c97a

return blsSig, true
}

// aggregateSignatures constructs a BLS aggregate signature from the collected validator signatures. Also
Expand Down
3 changes: 2 additions & 1 deletion signature-aggregator/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,13 @@ func signatureAggregationAPIHandler(logger logging.Logger, aggregator *aggregato
return
}

if isEmptyOrZeroes(message.Bytes()) || isEmptyOrZeroes(justification) {
if isEmptyOrZeroes(message.Bytes()) && isEmptyOrZeroes(justification) {
michaelkaplan13 marked this conversation as resolved.
Show resolved Hide resolved
writeJSONError(
logger,
w,
"Must provide either message or justification",
)
return
}

quorumPercentage := req.QuorumPercentage
Expand Down
2 changes: 1 addition & 1 deletion tests/contracts/lib/teleporter
Loading