From 72373fcad594d945179c1a941a11840b422b48f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Tue, 1 Sep 2026 15:33:55 +0000 Subject: [PATCH 1/9] feat(tbtc): switch reservation proposal marshaling to protobuf Closes gap-analysis Major row 1 and implementation-plan.md M1 row 3. ReservationAnchorProposal, ReservedRedemptionProposal, ReservationReanchorProposal, and ReservationDissolutionProposal previously used a JSON Marshal/Unmarshal placeholder, unlike every other CoordinationProposal type in this package (Heartbeat, DepositSweep, Redemption, MovingFunds, MovedFundsSweep), which all marshal via pkg/tbtc/gen/pb. Added the four missing message types to message.proto and regenerated message.pb.go (protoc 3.21.12 installed for this). Moved the four proposals' Marshal/Unmarshal from reservation.go's JSON stubs into marshaling.go, matching the existing proto-based implementations' structure and field-encoding conventions (big.Int fees via .Bytes()/SetBytes(), fixed-size hashes/pubkey-hashes via byte-slice copy with a length check). Preserved the original JSON stubs' validation intent under proto3's zero-value-is-absence semantics: a request nonce of 0, or empty fee/reservation-key/hash bytes, are rejected the same way an explicitly-missing JSON field was. The original '== nil' checks on *big.Int fields don't carry over as-is - SetBytes never returns nil - so they're now byte-length checks on the wire field instead, which is the pattern every other proto-based proposal in this file already uses. Testing: extended the existing table-driven TestCoordinationMessage_MarshalingRoundtrip with the four new types (exact field-for-field equality through the wire, matching the existing test's own precision, not just the fuzz-style tests already covering every sibling type) plus four new TestFuzzCoordinationMessage_MarshalingRoundtrip_WithProposal crash-safety tests, matching the one-per-type convention. Rewrote the pre-existing TestReservationProposals_UnmarshalRejectsMissingIntegers (now TestReservationProposals_UnmarshalRejectsInvalidFields) to construct real protobuf payloads instead of JSON string literals, porting every original missing-field case plus two new structural cases (invalid hash/pubkey-hash length) that fall out of the new wire format. go test ./pkg/tbtc/...: 15/15 new/changed tests pass, full package suite passes (146s), -race clean (156s). gofmt/vet clean on all 6 changed files. --- pkg/tbtc/gen/pb/message.pb.go | 397 ++++++++++++++++++++++++++++++++-- pkg/tbtc/gen/pb/message.proto | 26 +++ pkg/tbtc/marshaling.go | 180 +++++++++++++++ pkg/tbtc/marshaling_test.go | 146 +++++++++++++ pkg/tbtc/reservation.go | 114 ---------- pkg/tbtc/reservation_test.go | 121 ++++++++--- 6 files changed, 823 insertions(+), 161 deletions(-) diff --git a/pkg/tbtc/gen/pb/message.pb.go b/pkg/tbtc/gen/pb/message.pb.go index 7496ad009d..41a7f348df 100644 --- a/pkg/tbtc/gen/pb/message.pb.go +++ b/pkg/tbtc/gen/pb/message.pb.go @@ -1,7 +1,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: -// protoc-gen-go v1.28.0 -// protoc v3.19.4 +// protoc-gen-go v1.30.0 +// protoc v3.21.12 // source: pkg/tbtc/gen/pb/message.proto package pb @@ -508,6 +508,274 @@ func (x *MovedFundsSweepProposal) GetSweepTxFee() []byte { return nil } +type ReservationAnchorProposal struct { + state protoimpl.MessageState + sizeCache protoimpl.SizeCache + unknownFields protoimpl.UnknownFields + + DepositFundingTxHash []byte `protobuf:"bytes,1,opt,name=depositFundingTxHash,proto3" json:"depositFundingTxHash,omitempty"` + DepositFundingOutputIndex uint32 `protobuf:"varint,2,opt,name=depositFundingOutputIndex,proto3" json:"depositFundingOutputIndex,omitempty"` + RequestNonce uint64 `protobuf:"varint,3,opt,name=requestNonce,proto3" json:"requestNonce,omitempty"` + AnchorTxFee []byte `protobuf:"bytes,4,opt,name=anchorTxFee,proto3" json:"anchorTxFee,omitempty"` +} + +func (x *ReservationAnchorProposal) Reset() { + *x = ReservationAnchorProposal{} + if protoimpl.UnsafeEnabled { + mi := &file_pkg_tbtc_gen_pb_message_proto_msgTypes[8] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) + } +} + +func (x *ReservationAnchorProposal) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*ReservationAnchorProposal) ProtoMessage() {} + +func (x *ReservationAnchorProposal) ProtoReflect() protoreflect.Message { + mi := &file_pkg_tbtc_gen_pb_message_proto_msgTypes[8] + if protoimpl.UnsafeEnabled && x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use ReservationAnchorProposal.ProtoReflect.Descriptor instead. +func (*ReservationAnchorProposal) Descriptor() ([]byte, []int) { + return file_pkg_tbtc_gen_pb_message_proto_rawDescGZIP(), []int{8} +} + +func (x *ReservationAnchorProposal) GetDepositFundingTxHash() []byte { + if x != nil { + return x.DepositFundingTxHash + } + return nil +} + +func (x *ReservationAnchorProposal) GetDepositFundingOutputIndex() uint32 { + if x != nil { + return x.DepositFundingOutputIndex + } + return 0 +} + +func (x *ReservationAnchorProposal) GetRequestNonce() uint64 { + if x != nil { + return x.RequestNonce + } + return 0 +} + +func (x *ReservationAnchorProposal) GetAnchorTxFee() []byte { + if x != nil { + return x.AnchorTxFee + } + return nil +} + +type ReservedRedemptionProposal struct { + state protoimpl.MessageState + sizeCache protoimpl.SizeCache + unknownFields protoimpl.UnknownFields + + ReservationKey []byte `protobuf:"bytes,1,opt,name=reservationKey,proto3" json:"reservationKey,omitempty"` + RequestNonce uint64 `protobuf:"varint,2,opt,name=requestNonce,proto3" json:"requestNonce,omitempty"` + RedemptionTxFee []byte `protobuf:"bytes,3,opt,name=redemptionTxFee,proto3" json:"redemptionTxFee,omitempty"` +} + +func (x *ReservedRedemptionProposal) Reset() { + *x = ReservedRedemptionProposal{} + if protoimpl.UnsafeEnabled { + mi := &file_pkg_tbtc_gen_pb_message_proto_msgTypes[9] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) + } +} + +func (x *ReservedRedemptionProposal) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*ReservedRedemptionProposal) ProtoMessage() {} + +func (x *ReservedRedemptionProposal) ProtoReflect() protoreflect.Message { + mi := &file_pkg_tbtc_gen_pb_message_proto_msgTypes[9] + if protoimpl.UnsafeEnabled && x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use ReservedRedemptionProposal.ProtoReflect.Descriptor instead. +func (*ReservedRedemptionProposal) Descriptor() ([]byte, []int) { + return file_pkg_tbtc_gen_pb_message_proto_rawDescGZIP(), []int{9} +} + +func (x *ReservedRedemptionProposal) GetReservationKey() []byte { + if x != nil { + return x.ReservationKey + } + return nil +} + +func (x *ReservedRedemptionProposal) GetRequestNonce() uint64 { + if x != nil { + return x.RequestNonce + } + return 0 +} + +func (x *ReservedRedemptionProposal) GetRedemptionTxFee() []byte { + if x != nil { + return x.RedemptionTxFee + } + return nil +} + +type ReservationReanchorProposal struct { + state protoimpl.MessageState + sizeCache protoimpl.SizeCache + unknownFields protoimpl.UnknownFields + + ReservationKey []byte `protobuf:"bytes,1,opt,name=reservationKey,proto3" json:"reservationKey,omitempty"` + RequestNonce uint64 `protobuf:"varint,2,opt,name=requestNonce,proto3" json:"requestNonce,omitempty"` + TargetWalletPublicKeyHash []byte `protobuf:"bytes,3,opt,name=targetWalletPublicKeyHash,proto3" json:"targetWalletPublicKeyHash,omitempty"` + ReanchorTxFee []byte `protobuf:"bytes,4,opt,name=reanchorTxFee,proto3" json:"reanchorTxFee,omitempty"` +} + +func (x *ReservationReanchorProposal) Reset() { + *x = ReservationReanchorProposal{} + if protoimpl.UnsafeEnabled { + mi := &file_pkg_tbtc_gen_pb_message_proto_msgTypes[10] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) + } +} + +func (x *ReservationReanchorProposal) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*ReservationReanchorProposal) ProtoMessage() {} + +func (x *ReservationReanchorProposal) ProtoReflect() protoreflect.Message { + mi := &file_pkg_tbtc_gen_pb_message_proto_msgTypes[10] + if protoimpl.UnsafeEnabled && x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use ReservationReanchorProposal.ProtoReflect.Descriptor instead. +func (*ReservationReanchorProposal) Descriptor() ([]byte, []int) { + return file_pkg_tbtc_gen_pb_message_proto_rawDescGZIP(), []int{10} +} + +func (x *ReservationReanchorProposal) GetReservationKey() []byte { + if x != nil { + return x.ReservationKey + } + return nil +} + +func (x *ReservationReanchorProposal) GetRequestNonce() uint64 { + if x != nil { + return x.RequestNonce + } + return 0 +} + +func (x *ReservationReanchorProposal) GetTargetWalletPublicKeyHash() []byte { + if x != nil { + return x.TargetWalletPublicKeyHash + } + return nil +} + +func (x *ReservationReanchorProposal) GetReanchorTxFee() []byte { + if x != nil { + return x.ReanchorTxFee + } + return nil +} + +type ReservationDissolutionProposal struct { + state protoimpl.MessageState + sizeCache protoimpl.SizeCache + unknownFields protoimpl.UnknownFields + + ReservationKey []byte `protobuf:"bytes,1,opt,name=reservationKey,proto3" json:"reservationKey,omitempty"` + RequestNonce uint64 `protobuf:"varint,2,opt,name=requestNonce,proto3" json:"requestNonce,omitempty"` + DissolutionTxFee []byte `protobuf:"bytes,3,opt,name=dissolutionTxFee,proto3" json:"dissolutionTxFee,omitempty"` +} + +func (x *ReservationDissolutionProposal) Reset() { + *x = ReservationDissolutionProposal{} + if protoimpl.UnsafeEnabled { + mi := &file_pkg_tbtc_gen_pb_message_proto_msgTypes[11] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) + } +} + +func (x *ReservationDissolutionProposal) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*ReservationDissolutionProposal) ProtoMessage() {} + +func (x *ReservationDissolutionProposal) ProtoReflect() protoreflect.Message { + mi := &file_pkg_tbtc_gen_pb_message_proto_msgTypes[11] + if protoimpl.UnsafeEnabled && x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use ReservationDissolutionProposal.ProtoReflect.Descriptor instead. +func (*ReservationDissolutionProposal) Descriptor() ([]byte, []int) { + return file_pkg_tbtc_gen_pb_message_proto_rawDescGZIP(), []int{11} +} + +func (x *ReservationDissolutionProposal) GetReservationKey() []byte { + if x != nil { + return x.ReservationKey + } + return nil +} + +func (x *ReservationDissolutionProposal) GetRequestNonce() uint64 { + if x != nil { + return x.RequestNonce + } + return 0 +} + +func (x *ReservationDissolutionProposal) GetDissolutionTxFee() []byte { + if x != nil { + return x.DissolutionTxFee + } + return nil +} + type DepositSweepProposal_DepositKey struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -520,7 +788,7 @@ type DepositSweepProposal_DepositKey struct { func (x *DepositSweepProposal_DepositKey) Reset() { *x = DepositSweepProposal_DepositKey{} if protoimpl.UnsafeEnabled { - mi := &file_pkg_tbtc_gen_pb_message_proto_msgTypes[8] + mi := &file_pkg_tbtc_gen_pb_message_proto_msgTypes[12] ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) ms.StoreMessageInfo(mi) } @@ -533,7 +801,7 @@ func (x *DepositSweepProposal_DepositKey) String() string { func (*DepositSweepProposal_DepositKey) ProtoMessage() {} func (x *DepositSweepProposal_DepositKey) ProtoReflect() protoreflect.Message { - mi := &file_pkg_tbtc_gen_pb_message_proto_msgTypes[8] + mi := &file_pkg_tbtc_gen_pb_message_proto_msgTypes[12] if protoimpl.UnsafeEnabled && x != nil { ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) if ms.LoadMessageInfo() == nil { @@ -642,8 +910,53 @@ var file_pkg_tbtc_gen_pb_message_proto_rawDesc = []byte{ 0x46, 0x75, 0x6e, 0x64, 0x73, 0x54, 0x78, 0x4f, 0x75, 0x74, 0x70, 0x75, 0x74, 0x49, 0x6e, 0x64, 0x65, 0x78, 0x12, 0x1e, 0x0a, 0x0a, 0x73, 0x77, 0x65, 0x65, 0x70, 0x54, 0x78, 0x46, 0x65, 0x65, 0x18, 0x03, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x0a, 0x73, 0x77, 0x65, 0x65, 0x70, 0x54, 0x78, 0x46, - 0x65, 0x65, 0x42, 0x06, 0x5a, 0x04, 0x2e, 0x2f, 0x70, 0x62, 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, - 0x6f, 0x33, + 0x65, 0x65, 0x22, 0xd3, 0x01, 0x0a, 0x19, 0x52, 0x65, 0x73, 0x65, 0x72, 0x76, 0x61, 0x74, 0x69, + 0x6f, 0x6e, 0x41, 0x6e, 0x63, 0x68, 0x6f, 0x72, 0x50, 0x72, 0x6f, 0x70, 0x6f, 0x73, 0x61, 0x6c, + 0x12, 0x32, 0x0a, 0x14, 0x64, 0x65, 0x70, 0x6f, 0x73, 0x69, 0x74, 0x46, 0x75, 0x6e, 0x64, 0x69, + 0x6e, 0x67, 0x54, 0x78, 0x48, 0x61, 0x73, 0x68, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x14, + 0x64, 0x65, 0x70, 0x6f, 0x73, 0x69, 0x74, 0x46, 0x75, 0x6e, 0x64, 0x69, 0x6e, 0x67, 0x54, 0x78, + 0x48, 0x61, 0x73, 0x68, 0x12, 0x3c, 0x0a, 0x19, 0x64, 0x65, 0x70, 0x6f, 0x73, 0x69, 0x74, 0x46, + 0x75, 0x6e, 0x64, 0x69, 0x6e, 0x67, 0x4f, 0x75, 0x74, 0x70, 0x75, 0x74, 0x49, 0x6e, 0x64, 0x65, + 0x78, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0d, 0x52, 0x19, 0x64, 0x65, 0x70, 0x6f, 0x73, 0x69, 0x74, + 0x46, 0x75, 0x6e, 0x64, 0x69, 0x6e, 0x67, 0x4f, 0x75, 0x74, 0x70, 0x75, 0x74, 0x49, 0x6e, 0x64, + 0x65, 0x78, 0x12, 0x22, 0x0a, 0x0c, 0x72, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x4e, 0x6f, 0x6e, + 0x63, 0x65, 0x18, 0x03, 0x20, 0x01, 0x28, 0x04, 0x52, 0x0c, 0x72, 0x65, 0x71, 0x75, 0x65, 0x73, + 0x74, 0x4e, 0x6f, 0x6e, 0x63, 0x65, 0x12, 0x20, 0x0a, 0x0b, 0x61, 0x6e, 0x63, 0x68, 0x6f, 0x72, + 0x54, 0x78, 0x46, 0x65, 0x65, 0x18, 0x04, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x0b, 0x61, 0x6e, 0x63, + 0x68, 0x6f, 0x72, 0x54, 0x78, 0x46, 0x65, 0x65, 0x22, 0x92, 0x01, 0x0a, 0x1a, 0x52, 0x65, 0x73, + 0x65, 0x72, 0x76, 0x65, 0x64, 0x52, 0x65, 0x64, 0x65, 0x6d, 0x70, 0x74, 0x69, 0x6f, 0x6e, 0x50, + 0x72, 0x6f, 0x70, 0x6f, 0x73, 0x61, 0x6c, 0x12, 0x26, 0x0a, 0x0e, 0x72, 0x65, 0x73, 0x65, 0x72, + 0x76, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x4b, 0x65, 0x79, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0c, 0x52, + 0x0e, 0x72, 0x65, 0x73, 0x65, 0x72, 0x76, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x4b, 0x65, 0x79, 0x12, + 0x22, 0x0a, 0x0c, 0x72, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x4e, 0x6f, 0x6e, 0x63, 0x65, 0x18, + 0x02, 0x20, 0x01, 0x28, 0x04, 0x52, 0x0c, 0x72, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x4e, 0x6f, + 0x6e, 0x63, 0x65, 0x12, 0x28, 0x0a, 0x0f, 0x72, 0x65, 0x64, 0x65, 0x6d, 0x70, 0x74, 0x69, 0x6f, + 0x6e, 0x54, 0x78, 0x46, 0x65, 0x65, 0x18, 0x03, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x0f, 0x72, 0x65, + 0x64, 0x65, 0x6d, 0x70, 0x74, 0x69, 0x6f, 0x6e, 0x54, 0x78, 0x46, 0x65, 0x65, 0x22, 0xcd, 0x01, + 0x0a, 0x1b, 0x52, 0x65, 0x73, 0x65, 0x72, 0x76, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x52, 0x65, 0x61, + 0x6e, 0x63, 0x68, 0x6f, 0x72, 0x50, 0x72, 0x6f, 0x70, 0x6f, 0x73, 0x61, 0x6c, 0x12, 0x26, 0x0a, + 0x0e, 0x72, 0x65, 0x73, 0x65, 0x72, 0x76, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x4b, 0x65, 0x79, 0x18, + 0x01, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x0e, 0x72, 0x65, 0x73, 0x65, 0x72, 0x76, 0x61, 0x74, 0x69, + 0x6f, 0x6e, 0x4b, 0x65, 0x79, 0x12, 0x22, 0x0a, 0x0c, 0x72, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, + 0x4e, 0x6f, 0x6e, 0x63, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x04, 0x52, 0x0c, 0x72, 0x65, 0x71, + 0x75, 0x65, 0x73, 0x74, 0x4e, 0x6f, 0x6e, 0x63, 0x65, 0x12, 0x3c, 0x0a, 0x19, 0x74, 0x61, 0x72, + 0x67, 0x65, 0x74, 0x57, 0x61, 0x6c, 0x6c, 0x65, 0x74, 0x50, 0x75, 0x62, 0x6c, 0x69, 0x63, 0x4b, + 0x65, 0x79, 0x48, 0x61, 0x73, 0x68, 0x18, 0x03, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x19, 0x74, 0x61, + 0x72, 0x67, 0x65, 0x74, 0x57, 0x61, 0x6c, 0x6c, 0x65, 0x74, 0x50, 0x75, 0x62, 0x6c, 0x69, 0x63, + 0x4b, 0x65, 0x79, 0x48, 0x61, 0x73, 0x68, 0x12, 0x24, 0x0a, 0x0d, 0x72, 0x65, 0x61, 0x6e, 0x63, + 0x68, 0x6f, 0x72, 0x54, 0x78, 0x46, 0x65, 0x65, 0x18, 0x04, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x0d, + 0x72, 0x65, 0x61, 0x6e, 0x63, 0x68, 0x6f, 0x72, 0x54, 0x78, 0x46, 0x65, 0x65, 0x22, 0x98, 0x01, + 0x0a, 0x1e, 0x52, 0x65, 0x73, 0x65, 0x72, 0x76, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x44, 0x69, 0x73, + 0x73, 0x6f, 0x6c, 0x75, 0x74, 0x69, 0x6f, 0x6e, 0x50, 0x72, 0x6f, 0x70, 0x6f, 0x73, 0x61, 0x6c, + 0x12, 0x26, 0x0a, 0x0e, 0x72, 0x65, 0x73, 0x65, 0x72, 0x76, 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x4b, + 0x65, 0x79, 0x18, 0x01, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x0e, 0x72, 0x65, 0x73, 0x65, 0x72, 0x76, + 0x61, 0x74, 0x69, 0x6f, 0x6e, 0x4b, 0x65, 0x79, 0x12, 0x22, 0x0a, 0x0c, 0x72, 0x65, 0x71, 0x75, + 0x65, 0x73, 0x74, 0x4e, 0x6f, 0x6e, 0x63, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x04, 0x52, 0x0c, + 0x72, 0x65, 0x71, 0x75, 0x65, 0x73, 0x74, 0x4e, 0x6f, 0x6e, 0x63, 0x65, 0x12, 0x2a, 0x0a, 0x10, + 0x64, 0x69, 0x73, 0x73, 0x6f, 0x6c, 0x75, 0x74, 0x69, 0x6f, 0x6e, 0x54, 0x78, 0x46, 0x65, 0x65, + 0x18, 0x03, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x10, 0x64, 0x69, 0x73, 0x73, 0x6f, 0x6c, 0x75, 0x74, + 0x69, 0x6f, 0x6e, 0x54, 0x78, 0x46, 0x65, 0x65, 0x42, 0x06, 0x5a, 0x04, 0x2e, 0x2f, 0x70, 0x62, + 0x62, 0x06, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( @@ -658,7 +971,7 @@ func file_pkg_tbtc_gen_pb_message_proto_rawDescGZIP() []byte { return file_pkg_tbtc_gen_pb_message_proto_rawDescData } -var file_pkg_tbtc_gen_pb_message_proto_msgTypes = make([]protoimpl.MessageInfo, 9) +var file_pkg_tbtc_gen_pb_message_proto_msgTypes = make([]protoimpl.MessageInfo, 13) var file_pkg_tbtc_gen_pb_message_proto_goTypes = []interface{}{ (*SigningDoneMessage)(nil), // 0: tbtc.SigningDoneMessage (*CoordinationProposal)(nil), // 1: tbtc.CoordinationProposal @@ -668,16 +981,20 @@ var file_pkg_tbtc_gen_pb_message_proto_goTypes = []interface{}{ (*RedemptionProposal)(nil), // 5: tbtc.RedemptionProposal (*MovingFundsProposal)(nil), // 6: tbtc.MovingFundsProposal (*MovedFundsSweepProposal)(nil), // 7: tbtc.MovedFundsSweepProposal - (*DepositSweepProposal_DepositKey)(nil), // 8: tbtc.DepositSweepProposal.DepositKey + (*ReservationAnchorProposal)(nil), // 8: tbtc.ReservationAnchorProposal + (*ReservedRedemptionProposal)(nil), // 9: tbtc.ReservedRedemptionProposal + (*ReservationReanchorProposal)(nil), // 10: tbtc.ReservationReanchorProposal + (*ReservationDissolutionProposal)(nil), // 11: tbtc.ReservationDissolutionProposal + (*DepositSweepProposal_DepositKey)(nil), // 12: tbtc.DepositSweepProposal.DepositKey } var file_pkg_tbtc_gen_pb_message_proto_depIdxs = []int32{ - 1, // 0: tbtc.CoordinationMessage.proposal:type_name -> tbtc.CoordinationProposal - 8, // 1: tbtc.DepositSweepProposal.depositsKeys:type_name -> tbtc.DepositSweepProposal.DepositKey - 2, // [2:2] is the sub-list for method output_type - 2, // [2:2] is the sub-list for method input_type - 2, // [2:2] is the sub-list for extension type_name - 2, // [2:2] is the sub-list for extension extendee - 0, // [0:2] is the sub-list for field type_name + 1, // 0: tbtc.CoordinationMessage.proposal:type_name -> tbtc.CoordinationProposal + 12, // 1: tbtc.DepositSweepProposal.depositsKeys:type_name -> tbtc.DepositSweepProposal.DepositKey + 2, // [2:2] is the sub-list for method output_type + 2, // [2:2] is the sub-list for method input_type + 2, // [2:2] is the sub-list for extension type_name + 2, // [2:2] is the sub-list for extension extendee + 0, // [0:2] is the sub-list for field type_name } func init() { file_pkg_tbtc_gen_pb_message_proto_init() } @@ -783,6 +1100,54 @@ func file_pkg_tbtc_gen_pb_message_proto_init() { } } file_pkg_tbtc_gen_pb_message_proto_msgTypes[8].Exporter = func(v interface{}, i int) interface{} { + switch v := v.(*ReservationAnchorProposal); i { + case 0: + return &v.state + case 1: + return &v.sizeCache + case 2: + return &v.unknownFields + default: + return nil + } + } + file_pkg_tbtc_gen_pb_message_proto_msgTypes[9].Exporter = func(v interface{}, i int) interface{} { + switch v := v.(*ReservedRedemptionProposal); i { + case 0: + return &v.state + case 1: + return &v.sizeCache + case 2: + return &v.unknownFields + default: + return nil + } + } + file_pkg_tbtc_gen_pb_message_proto_msgTypes[10].Exporter = func(v interface{}, i int) interface{} { + switch v := v.(*ReservationReanchorProposal); i { + case 0: + return &v.state + case 1: + return &v.sizeCache + case 2: + return &v.unknownFields + default: + return nil + } + } + file_pkg_tbtc_gen_pb_message_proto_msgTypes[11].Exporter = func(v interface{}, i int) interface{} { + switch v := v.(*ReservationDissolutionProposal); i { + case 0: + return &v.state + case 1: + return &v.sizeCache + case 2: + return &v.unknownFields + default: + return nil + } + } + file_pkg_tbtc_gen_pb_message_proto_msgTypes[12].Exporter = func(v interface{}, i int) interface{} { switch v := v.(*DepositSweepProposal_DepositKey); i { case 0: return &v.state @@ -801,7 +1166,7 @@ func file_pkg_tbtc_gen_pb_message_proto_init() { GoPackagePath: reflect.TypeOf(x{}).PkgPath(), RawDescriptor: file_pkg_tbtc_gen_pb_message_proto_rawDesc, NumEnums: 0, - NumMessages: 9, + NumMessages: 13, NumExtensions: 0, NumServices: 0, }, diff --git a/pkg/tbtc/gen/pb/message.proto b/pkg/tbtc/gen/pb/message.proto index e26d31ab46..6e5c3eafa3 100644 --- a/pkg/tbtc/gen/pb/message.proto +++ b/pkg/tbtc/gen/pb/message.proto @@ -53,3 +53,29 @@ message MovedFundsSweepProposal { uint32 movingFundsTxOutputIndex = 2; bytes sweepTxFee = 3; } + +message ReservationAnchorProposal { + bytes depositFundingTxHash = 1; + uint32 depositFundingOutputIndex = 2; + uint64 requestNonce = 3; + bytes anchorTxFee = 4; +} + +message ReservedRedemptionProposal { + bytes reservationKey = 1; + uint64 requestNonce = 2; + bytes redemptionTxFee = 3; +} + +message ReservationReanchorProposal { + bytes reservationKey = 1; + uint64 requestNonce = 2; + bytes targetWalletPublicKeyHash = 3; + bytes reanchorTxFee = 4; +} + +message ReservationDissolutionProposal { + bytes reservationKey = 1; + uint64 requestNonce = 2; + bytes dissolutionTxFee = 3; +} diff --git a/pkg/tbtc/marshaling.go b/pkg/tbtc/marshaling.go index d31180ca27..6f31ff6c21 100644 --- a/pkg/tbtc/marshaling.go +++ b/pkg/tbtc/marshaling.go @@ -470,6 +470,186 @@ func (mfsp *MovedFundsSweepProposal) Unmarshal(data []byte) error { return nil } +// Marshal converts the reservationAnchorProposal to a byte array. +func (rap *ReservationAnchorProposal) Marshal() ([]byte, error) { + return proto.Marshal( + &pb.ReservationAnchorProposal{ + DepositFundingTxHash: rap.DepositFundingTxHash[:], + DepositFundingOutputIndex: rap.DepositFundingOutputIndex, + RequestNonce: rap.RequestNonce, + AnchorTxFee: rap.AnchorTxFee.Bytes(), + }, + ) +} + +// Unmarshal converts a byte array back to the reservationAnchorProposal. +func (rap *ReservationAnchorProposal) Unmarshal(bytes []byte) error { + pbMsg := pb.ReservationAnchorProposal{} + if err := proto.Unmarshal(bytes, &pbMsg); err != nil { + return fmt.Errorf("failed to unmarshal ReservationAnchorProposal: [%v]", err) + } + + if len(pbMsg.DepositFundingTxHash) != 32 { + return fmt.Errorf( + "invalid deposit funding tx hash length: [%v]", + len(pbMsg.DepositFundingTxHash), + ) + } + // RequestNonce identifies the acceptance authorization generation this + // proposal targets; 0 is the reservation's default zero-value meaning + // "no action requested", so a proposal must never carry it. + if pbMsg.RequestNonce == 0 { + return fmt.Errorf("request nonce is required") + } + // Proto3 scalar fields have no wire presence: an omitted AnchorTxFee + // and an explicit zero-value one are indistinguishable on the wire. + // A zero fee is never valid for a real anchor transaction, so treat + // it the same as "missing" - this mirrors the original JSON-based + // Unmarshal's "anchor transaction fee is required" guard. + if len(pbMsg.AnchorTxFee) == 0 { + return fmt.Errorf("anchor transaction fee is required") + } + + var depositFundingTxHash bitcoin.Hash + copy(depositFundingTxHash[:], pbMsg.DepositFundingTxHash) + + rap.DepositFundingTxHash = depositFundingTxHash + rap.DepositFundingOutputIndex = pbMsg.DepositFundingOutputIndex + rap.RequestNonce = pbMsg.RequestNonce + rap.AnchorTxFee = new(big.Int).SetBytes(pbMsg.AnchorTxFee) + + return nil +} + +// Marshal converts the reservedRedemptionProposal to a byte array. +func (rrp *ReservedRedemptionProposal) Marshal() ([]byte, error) { + return proto.Marshal( + &pb.ReservedRedemptionProposal{ + ReservationKey: rrp.ReservationKey.Bytes(), + RequestNonce: rrp.RequestNonce, + RedemptionTxFee: rrp.RedemptionTxFee.Bytes(), + }, + ) +} + +// Unmarshal converts a byte array back to the reservedRedemptionProposal. +func (rrp *ReservedRedemptionProposal) Unmarshal(bytes []byte) error { + pbMsg := pb.ReservedRedemptionProposal{} + if err := proto.Unmarshal(bytes, &pbMsg); err != nil { + return fmt.Errorf("failed to unmarshal ReservedRedemptionProposal: [%v]", err) + } + + // See ReservationAnchorProposal.Unmarshal: proto3 zero-value fields + // are indistinguishable from omitted ones, so a zero reservation key + // (never legitimate - keys are derived hashes) is treated as missing, + // mirroring the original JSON-based Unmarshal's guard. + if len(pbMsg.ReservationKey) == 0 { + return fmt.Errorf("reservation key is required") + } + // See the comment in ReservationAnchorProposal.Unmarshal: nonce 0 is + // the reservation's "no action requested" zero-value. + if pbMsg.RequestNonce == 0 { + return fmt.Errorf("request nonce is required") + } + if len(pbMsg.RedemptionTxFee) == 0 { + return fmt.Errorf("redemption transaction fee is required") + } + + rrp.ReservationKey = new(big.Int).SetBytes(pbMsg.ReservationKey) + rrp.RequestNonce = pbMsg.RequestNonce + rrp.RedemptionTxFee = new(big.Int).SetBytes(pbMsg.RedemptionTxFee) + + return nil +} + +// Marshal converts the reservationReanchorProposal to a byte array. +func (rrp *ReservationReanchorProposal) Marshal() ([]byte, error) { + return proto.Marshal( + &pb.ReservationReanchorProposal{ + ReservationKey: rrp.ReservationKey.Bytes(), + RequestNonce: rrp.RequestNonce, + TargetWalletPublicKeyHash: rrp.TargetWalletPublicKeyHash[:], + ReanchorTxFee: rrp.ReanchorTxFee.Bytes(), + }, + ) +} + +// Unmarshal converts a byte array back to the reservationReanchorProposal. +func (rrp *ReservationReanchorProposal) Unmarshal(bytes []byte) error { + pbMsg := pb.ReservationReanchorProposal{} + if err := proto.Unmarshal(bytes, &pbMsg); err != nil { + return fmt.Errorf("failed to unmarshal ReservationReanchorProposal: [%v]", err) + } + + if len(pbMsg.ReservationKey) == 0 { + return fmt.Errorf("reservation key is required") + } + if len(pbMsg.TargetWalletPublicKeyHash) != 20 { + return fmt.Errorf( + "invalid target wallet public key hash length: [%v]", + len(pbMsg.TargetWalletPublicKeyHash), + ) + } + // See the comment in ReservationAnchorProposal.Unmarshal: nonce 0 is + // the reservation's "no action requested" zero-value. + if pbMsg.RequestNonce == 0 { + return fmt.Errorf("request nonce is required") + } + if len(pbMsg.ReanchorTxFee) == 0 { + return fmt.Errorf("re-anchor transaction fee is required") + } + + var targetWalletPublicKeyHash [20]byte + copy(targetWalletPublicKeyHash[:], pbMsg.TargetWalletPublicKeyHash) + + rrp.ReservationKey = new(big.Int).SetBytes(pbMsg.ReservationKey) + rrp.RequestNonce = pbMsg.RequestNonce + rrp.TargetWalletPublicKeyHash = targetWalletPublicKeyHash + rrp.ReanchorTxFee = new(big.Int).SetBytes(pbMsg.ReanchorTxFee) + + return nil +} + +// Marshal converts the reservationDissolutionProposal to a byte array. +func (rdp *ReservationDissolutionProposal) Marshal() ([]byte, error) { + return proto.Marshal( + &pb.ReservationDissolutionProposal{ + ReservationKey: rdp.ReservationKey.Bytes(), + RequestNonce: rdp.RequestNonce, + DissolutionTxFee: rdp.DissolutionTxFee.Bytes(), + }, + ) +} + +// Unmarshal converts a byte array back to the reservationDissolutionProposal. +func (rdp *ReservationDissolutionProposal) Unmarshal(bytes []byte) error { + pbMsg := pb.ReservationDissolutionProposal{} + if err := proto.Unmarshal(bytes, &pbMsg); err != nil { + return fmt.Errorf( + "failed to unmarshal ReservationDissolutionProposal: [%v]", + err, + ) + } + + if len(pbMsg.ReservationKey) == 0 { + return fmt.Errorf("reservation key is required") + } + // See the comment in ReservationAnchorProposal.Unmarshal: nonce 0 is + // the reservation's "no action requested" zero-value. + if pbMsg.RequestNonce == 0 { + return fmt.Errorf("request nonce is required") + } + if len(pbMsg.DissolutionTxFee) == 0 { + return fmt.Errorf("dissolution transaction fee is required") + } + + rdp.ReservationKey = new(big.Int).SetBytes(pbMsg.ReservationKey) + rdp.RequestNonce = pbMsg.RequestNonce + rdp.DissolutionTxFee = new(big.Int).SetBytes(pbMsg.DissolutionTxFee) + + return nil +} + // marshalPublicKey converts an ECDSA public key to a byte // array (uncompressed). func marshalPublicKey(publicKey *ecdsa.PublicKey) ([]byte, error) { diff --git a/pkg/tbtc/marshaling_test.go b/pkg/tbtc/marshaling_test.go index 32b6977f0a..3a6c61aef5 100644 --- a/pkg/tbtc/marshaling_test.go +++ b/pkg/tbtc/marshaling_test.go @@ -231,6 +231,36 @@ func TestCoordinationMessage_MarshalingRoundtrip(t *testing.T) { SweepTxFee: big.NewInt(8000), }, }, + "with reservation anchor proposal": { + proposal: &ReservationAnchorProposal{ + DepositFundingTxHash: parseHash("709b55bd3da0f5a838125bd0ee20c5bfdd7caba173912d4281cae816b79a201b"), + DepositFundingOutputIndex: 2, + RequestNonce: 7, + AnchorTxFee: big.NewInt(1500), + }, + }, + "with reserved redemption proposal": { + proposal: &ReservedRedemptionProposal{ + ReservationKey: big.NewInt(424242), + RequestNonce: 3, + RedemptionTxFee: big.NewInt(9000), + }, + }, + "with reservation reanchor proposal": { + proposal: &ReservationReanchorProposal{ + ReservationKey: big.NewInt(424242), + RequestNonce: 4, + TargetWalletPublicKeyHash: toByte20("f87eb7ec3b15a3fdd7b57754d765694b3e0b4bf4"), + ReanchorTxFee: big.NewInt(1200), + }, + }, + "with reservation dissolution proposal": { + proposal: &ReservationDissolutionProposal{ + ReservationKey: big.NewInt(424242), + RequestNonce: 5, + DissolutionTxFee: big.NewInt(1100), + }, + }, } walletPublicKeyHash := toByte20("aa768412ceed10bd423c025542ca90071f9fb62d") @@ -402,6 +432,122 @@ func TestFuzzCoordinationMessage_MarshalingRoundtrip_WithMovedFundsSweepProposal } } +func TestFuzzCoordinationMessage_MarshalingRoundtrip_WithReservationAnchorProposal(t *testing.T) { + for range 10 { + var ( + senderID group.MemberIndex + coordinationBlock uint64 + walletPublicKeyHash [20]byte + proposal ReservationAnchorProposal + ) + + f := fuzz.New().NilChance(0.1). + NumElements(0, 512). + Funcs(pbutils.FuzzFuncs()...) + + f.Fuzz(&senderID) + f.Fuzz(&coordinationBlock) + f.Fuzz(&walletPublicKeyHash) + f.Fuzz(&proposal) + + coordinationMsg := &coordinationMessage{ + senderID: senderID, + coordinationBlock: coordinationBlock, + walletPublicKeyHash: walletPublicKeyHash, + proposal: &proposal, + } + + _ = pbutils.RoundTrip(coordinationMsg, &coordinationMessage{}) + } +} + +func TestFuzzCoordinationMessage_MarshalingRoundtrip_WithReservedRedemptionProposal(t *testing.T) { + for range 10 { + var ( + senderID group.MemberIndex + coordinationBlock uint64 + walletPublicKeyHash [20]byte + proposal ReservedRedemptionProposal + ) + + f := fuzz.New().NilChance(0.1). + NumElements(0, 512). + Funcs(pbutils.FuzzFuncs()...) + + f.Fuzz(&senderID) + f.Fuzz(&coordinationBlock) + f.Fuzz(&walletPublicKeyHash) + f.Fuzz(&proposal) + + coordinationMsg := &coordinationMessage{ + senderID: senderID, + coordinationBlock: coordinationBlock, + walletPublicKeyHash: walletPublicKeyHash, + proposal: &proposal, + } + + _ = pbutils.RoundTrip(coordinationMsg, &coordinationMessage{}) + } +} + +func TestFuzzCoordinationMessage_MarshalingRoundtrip_WithReservationReanchorProposal(t *testing.T) { + for range 10 { + var ( + senderID group.MemberIndex + coordinationBlock uint64 + walletPublicKeyHash [20]byte + proposal ReservationReanchorProposal + ) + + f := fuzz.New().NilChance(0.1). + NumElements(0, 512). + Funcs(pbutils.FuzzFuncs()...) + + f.Fuzz(&senderID) + f.Fuzz(&coordinationBlock) + f.Fuzz(&walletPublicKeyHash) + f.Fuzz(&proposal) + + coordinationMsg := &coordinationMessage{ + senderID: senderID, + coordinationBlock: coordinationBlock, + walletPublicKeyHash: walletPublicKeyHash, + proposal: &proposal, + } + + _ = pbutils.RoundTrip(coordinationMsg, &coordinationMessage{}) + } +} + +func TestFuzzCoordinationMessage_MarshalingRoundtrip_WithReservationDissolutionProposal(t *testing.T) { + for range 10 { + var ( + senderID group.MemberIndex + coordinationBlock uint64 + walletPublicKeyHash [20]byte + proposal ReservationDissolutionProposal + ) + + f := fuzz.New().NilChance(0.1). + NumElements(0, 512). + Funcs(pbutils.FuzzFuncs()...) + + f.Fuzz(&senderID) + f.Fuzz(&coordinationBlock) + f.Fuzz(&walletPublicKeyHash) + f.Fuzz(&proposal) + + coordinationMsg := &coordinationMessage{ + senderID: senderID, + coordinationBlock: coordinationBlock, + walletPublicKeyHash: walletPublicKeyHash, + proposal: &proposal, + } + + _ = pbutils.RoundTrip(coordinationMsg, &coordinationMessage{}) + } +} + func TestFuzzCoordinationMessage_MarshalingRoundtrip_WithNoopProposal(t *testing.T) { for i := 0; i < 10; i++ { var ( diff --git a/pkg/tbtc/reservation.go b/pkg/tbtc/reservation.go index a7a4de2577..9e3b419afb 100644 --- a/pkg/tbtc/reservation.go +++ b/pkg/tbtc/reservation.go @@ -1,7 +1,6 @@ package tbtc import ( - "encoding/json" "fmt" "math/big" @@ -202,32 +201,6 @@ func (rap *ReservationAnchorProposal) ValidityBlocks() uint64 { return reservationAnchorProposalValidityBlocks } -// Marshal converts the reservationAnchorProposal to a byte array. -// -// TODO: Switch to protobuf-based marshaling (see pkg/tbtc/gen/pb) once the -// reservation message types are added to the coordination proto definition. -func (rap *ReservationAnchorProposal) Marshal() ([]byte, error) { - return json.Marshal(rap) -} - -// Unmarshal converts a byte array back to the reservationAnchorProposal. -func (rap *ReservationAnchorProposal) Unmarshal(bytes []byte) error { - var proposal ReservationAnchorProposal - if err := json.Unmarshal(bytes, &proposal); err != nil { - return err - } - if proposal.AnchorTxFee == nil { - return fmt.Errorf("anchor transaction fee is required") - } - if proposal.RequestNonce == 0 { - return fmt.Errorf("request nonce is required") - } - - *rap = proposal - - return nil -} - // ReservedRedemptionProposal represents a reserved redemption proposal // issued by a wallet's coordination leader. type ReservedRedemptionProposal struct { @@ -252,35 +225,6 @@ func (rrp *ReservedRedemptionProposal) ValidityBlocks() uint64 { return reservedRedemptionProposalValidityBlocks } -// Marshal converts the reservedRedemptionProposal to a byte array. -// -// TODO: Switch to protobuf-based marshaling (see pkg/tbtc/gen/pb) once the -// reservation message types are added to the coordination proto definition. -func (rrp *ReservedRedemptionProposal) Marshal() ([]byte, error) { - return json.Marshal(rrp) -} - -// Unmarshal converts a byte array back to the reservedRedemptionProposal. -func (rrp *ReservedRedemptionProposal) Unmarshal(bytes []byte) error { - var proposal ReservedRedemptionProposal - if err := json.Unmarshal(bytes, &proposal); err != nil { - return err - } - if proposal.ReservationKey == nil { - return fmt.Errorf("reservation key is required") - } - if proposal.RequestNonce == 0 { - return fmt.Errorf("request nonce is required") - } - if proposal.RedemptionTxFee == nil { - return fmt.Errorf("redemption transaction fee is required") - } - - *rrp = proposal - - return nil -} - // ReservationReanchorProposal represents a reservation re-anchor proposal // issued by a wallet's coordination leader, moving a reservation's anchor // outpoint to another wallet (e.g. during wallet migration). @@ -307,35 +251,6 @@ func (rrp *ReservationReanchorProposal) ValidityBlocks() uint64 { return reservationReanchorProposalValidityBlocks } -// Marshal converts the reservationReanchorProposal to a byte array. -// -// TODO: Switch to protobuf-based marshaling (see pkg/tbtc/gen/pb) once the -// reservation message types are added to the coordination proto definition. -func (rrp *ReservationReanchorProposal) Marshal() ([]byte, error) { - return json.Marshal(rrp) -} - -// Unmarshal converts a byte array back to the reservationReanchorProposal. -func (rrp *ReservationReanchorProposal) Unmarshal(bytes []byte) error { - var proposal ReservationReanchorProposal - if err := json.Unmarshal(bytes, &proposal); err != nil { - return err - } - if proposal.ReservationKey == nil { - return fmt.Errorf("reservation key is required") - } - if proposal.RequestNonce == 0 { - return fmt.Errorf("request nonce is required") - } - if proposal.ReanchorTxFee == nil { - return fmt.Errorf("re-anchor transaction fee is required") - } - - *rrp = proposal - - return nil -} - // ReservationDissolutionProposal represents a reservation dissolution // proposal issued by a wallet's coordination leader once the reservation's // custody term and grace period elapsed. @@ -360,35 +275,6 @@ func (rdp *ReservationDissolutionProposal) ValidityBlocks() uint64 { return reservationDissolutionProposalValidityBlocks } -// Marshal converts the reservationDissolutionProposal to a byte array. -// -// TODO: Switch to protobuf-based marshaling (see pkg/tbtc/gen/pb) once the -// reservation message types are added to the coordination proto definition. -func (rdp *ReservationDissolutionProposal) Marshal() ([]byte, error) { - return json.Marshal(rdp) -} - -// Unmarshal converts a byte array back to the reservationDissolutionProposal. -func (rdp *ReservationDissolutionProposal) Unmarshal(bytes []byte) error { - var proposal ReservationDissolutionProposal - if err := json.Unmarshal(bytes, &proposal); err != nil { - return err - } - if proposal.ReservationKey == nil { - return fmt.Errorf("reservation key is required") - } - if proposal.RequestNonce == 0 { - return fmt.Errorf("request nonce is required") - } - if proposal.DissolutionTxFee == nil { - return fmt.Errorf("dissolution transaction fee is required") - } - - *rdp = proposal - - return nil -} - // assembleReservationAnchorTransaction constructs an unsigned reservation // anchor transaction: a 1-input-1-output spend of the given reserved deposit // into a fresh output controlled by the given wallet. The anchor mirrors the diff --git a/pkg/tbtc/reservation_test.go b/pkg/tbtc/reservation_test.go index 16ab16c160..5deba71162 100644 --- a/pkg/tbtc/reservation_test.go +++ b/pkg/tbtc/reservation_test.go @@ -7,7 +7,10 @@ import ( "reflect" "testing" + "google.golang.org/protobuf/proto" + "github.com/keep-network/keep-core/pkg/bitcoin" + "github.com/keep-network/keep-core/pkg/tbtc/gen/pb" ) func TestReservationActionTypes(t *testing.T) { @@ -146,70 +149,126 @@ func TestReservationProposals_MarshalingRoundtrip(t *testing.T) { roundtrip(dissolutionProposal, &ReservationDissolutionProposal{}) } -func TestReservationProposals_UnmarshalRejectsMissingIntegers(t *testing.T) { +func TestReservationProposals_UnmarshalRejectsInvalidFields(t *testing.T) { + // marshalPb encodes an arbitrary protobuf message the same way + // proto.Marshal would, for building deliberately incomplete/invalid + // wire payloads. mustMarshal panics on error since every message + // here is well-formed at the protobuf level - only the domain-level + // validation performed by each proposal's Unmarshal is under test. + marshalPb := func(msg proto.Message) []byte { + bytes, err := proto.Marshal(msg) + if err != nil { + t.Fatal(err) + } + return bytes + } + + validHash := make([]byte, 32) + validHash[0] = 0x01 + validWalletHash := make([]byte, 20) + validWalletHash[0] = 0xaa + tests := map[string]struct { actionType WalletActionType - payload string + payload []byte expectedError string }{ - "anchor empty object": { + // Proto3 scalar fields have no wire presence, so an entirely + // empty payload and one with every field explicitly zeroed are + // indistinguishable - a single "empty payload" case per type + // covers what the old JSON test split into "empty object" and + // "null payload" cases. + "anchor empty payload": { actionType: ActionReservationAnchor, - payload: `{}`, - expectedError: "cannot unmarshal proposal payload: [anchor transaction fee is required]", - }, - "anchor null payload": { - actionType: ActionReservationAnchor, - payload: `null`, - expectedError: "cannot unmarshal proposal payload: [anchor transaction fee is required]", + payload: marshalPb(&pb.ReservationAnchorProposal{}), + expectedError: "cannot unmarshal proposal payload: [invalid deposit funding tx hash length: [0]]", }, "anchor missing nonce": { - actionType: ActionReservationAnchor, - payload: `{"AnchorTxFee":1500}`, + actionType: ActionReservationAnchor, + payload: marshalPb(&pb.ReservationAnchorProposal{ + DepositFundingTxHash: validHash, + AnchorTxFee: big.NewInt(1500).Bytes(), + }), expectedError: "cannot unmarshal proposal payload: [request nonce is required]", }, - "reserved redemption null payload": { + "anchor missing fee": { + actionType: ActionReservationAnchor, + payload: marshalPb(&pb.ReservationAnchorProposal{ + DepositFundingTxHash: validHash, + RequestNonce: 1, + }), + expectedError: "cannot unmarshal proposal payload: [anchor transaction fee is required]", + }, + "reserved redemption empty payload": { actionType: ActionReservedRedemption, - payload: `null`, + payload: marshalPb(&pb.ReservedRedemptionProposal{}), expectedError: "cannot unmarshal proposal payload: [reservation key is required]", }, "reserved redemption missing nonce": { - actionType: ActionReservedRedemption, - payload: `{"ReservationKey":12345,"RedemptionTxFee":1600}`, + actionType: ActionReservedRedemption, + payload: marshalPb(&pb.ReservedRedemptionProposal{ + ReservationKey: big.NewInt(12345).Bytes(), + RedemptionTxFee: big.NewInt(1600).Bytes(), + }), expectedError: "cannot unmarshal proposal payload: [request nonce is required]", }, "reserved redemption missing fee": { - actionType: ActionReservedRedemption, - payload: `{"ReservationKey":12345,"RequestNonce":2}`, + actionType: ActionReservedRedemption, + payload: marshalPb(&pb.ReservedRedemptionProposal{ + ReservationKey: big.NewInt(12345).Bytes(), + RequestNonce: 2, + }), expectedError: "cannot unmarshal proposal payload: [redemption transaction fee is required]", }, - "re-anchor null payload": { + "re-anchor empty payload": { actionType: ActionReservationReanchor, - payload: `null`, + payload: marshalPb(&pb.ReservationReanchorProposal{}), expectedError: "cannot unmarshal proposal payload: [reservation key is required]", }, + "re-anchor invalid target wallet hash length": { + actionType: ActionReservationReanchor, + payload: marshalPb(&pb.ReservationReanchorProposal{ + ReservationKey: big.NewInt(54321).Bytes(), + }), + expectedError: "cannot unmarshal proposal payload: [invalid target wallet public key hash length: [0]]", + }, "re-anchor missing nonce": { - actionType: ActionReservationReanchor, - payload: `{"ReservationKey":54321,"ReanchorTxFee":1700}`, + actionType: ActionReservationReanchor, + payload: marshalPb(&pb.ReservationReanchorProposal{ + ReservationKey: big.NewInt(54321).Bytes(), + TargetWalletPublicKeyHash: validWalletHash, + ReanchorTxFee: big.NewInt(1700).Bytes(), + }), expectedError: "cannot unmarshal proposal payload: [request nonce is required]", }, "re-anchor missing fee": { - actionType: ActionReservationReanchor, - payload: `{"ReservationKey":54321,"RequestNonce":3}`, + actionType: ActionReservationReanchor, + payload: marshalPb(&pb.ReservationReanchorProposal{ + ReservationKey: big.NewInt(54321).Bytes(), + TargetWalletPublicKeyHash: validWalletHash, + RequestNonce: 3, + }), expectedError: "cannot unmarshal proposal payload: [re-anchor transaction fee is required]", }, - "dissolution null payload": { + "dissolution empty payload": { actionType: ActionReservationDissolution, - payload: `null`, + payload: marshalPb(&pb.ReservationDissolutionProposal{}), expectedError: "cannot unmarshal proposal payload: [reservation key is required]", }, "dissolution missing nonce": { - actionType: ActionReservationDissolution, - payload: `{"ReservationKey":99999,"DissolutionTxFee":1800}`, + actionType: ActionReservationDissolution, + payload: marshalPb(&pb.ReservationDissolutionProposal{ + ReservationKey: big.NewInt(99999).Bytes(), + DissolutionTxFee: big.NewInt(1800).Bytes(), + }), expectedError: "cannot unmarshal proposal payload: [request nonce is required]", }, "dissolution missing fee": { - actionType: ActionReservationDissolution, - payload: `{"ReservationKey":99999,"RequestNonce":4}`, + actionType: ActionReservationDissolution, + payload: marshalPb(&pb.ReservationDissolutionProposal{ + ReservationKey: big.NewInt(99999).Bytes(), + RequestNonce: 4, + }), expectedError: "cannot unmarshal proposal payload: [dissolution transaction fee is required]", }, } @@ -218,7 +277,7 @@ func TestReservationProposals_UnmarshalRejectsMissingIntegers(t *testing.T) { t.Run(testName, func(t *testing.T) { _, err := unmarshalCoordinationProposal( uint32(test.actionType), - []byte(test.payload), + test.payload, ) if err == nil || err.Error() != test.expectedError { t.Errorf( From c8eb5419f6a46ed1f72b059825f9c1e698070c43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Tue, 1 Sep 2026 15:44:02 +0000 Subject: [PATCH 2/9] fix(tbtc): wire reservation actions into the coordination checklist Blocker gap found while preparing the M3 multi-signer integration test: pkg/tbtc/coordination.go's getActionsChecklist decides which WalletActionTypes a coordination round even considers, and it never emitted ActionReservationAnchor or ActionReservationReanchor. pkg/tbtcpg.ProposalGenerator.Generate only runs a task whose ActionType() appears in the checklist it's handed (tbtcpg.go:124-135) - it never iterates pg.tasks directly. NewReservationAcceptanceTask and NewReservationReanchorTask are registered when config.Reservations.Enabled=true (tbtcpg.go:88-92), but with no checklist entry, Generate's per-window loop never selected them. Both tasks were structurally unreachable in production regardless of PR #4276/#4277's fixes. Every existing unit test for these tasks (reservation_acceptance_test.go, reservation_reanchor_test.go) calls task.Run(request) directly, bypassing getActionsChecklist/Generate entirely - which is why this was never caught by any prior PR's test suite. Fix: ActionReservationAnchor and ActionReservationReanchor are now appended unconditionally, checked on every coordination window like ActionRedemption (both are custody-critical - an unaccepted reservation or a stale re-anchor risks stranding, not just reduced throughput - unlike the frequency-gated sweep/moving-funds actions). A node with reservations disabled safely no-ops on these: Generate already treats a checklist action with no matching registered task as 'unsupported' and skips it without error (tbtcpg.go:131-135), the same mechanism that already gates every other optional per-node task. Testing: - Updated TestCoordinationExecutor_GetActionsChecklist and its _PostActivation sibling: every non-nil expected checklist now includes both new actions right after ActionRedemption, matching the real append order. Extended assertChecklistOrdering's priority map accordingly (Redemption=0, ReservationAnchor=1, ReservationReanchor=2, then the existing sweep/moving-funds/ heartbeat priorities shifted). - Added TestCoordinationExecutor_GetActionsChecklist_ReservationActionsAlwaysPresent, a dedicated regression guard asserting both actions are present across pre/post-activation and 4th/non-4th windows, decoupled from the large table-driven test - would fail on its own if this wiring regresses. - go test ./pkg/tbtc/... ./pkg/tbtcpg/...: 476/476 pass. - go build ./... && go test ./...: full repo, 49 packages, zero FAIL. - gofmt/vet clean on both changed files. --- pkg/tbtc/coordination.go | 16 ++++- pkg/tbtc/coordination_test.go | 117 ++++++++++++++++++++++++++++------ 2 files changed, 112 insertions(+), 21 deletions(-) diff --git a/pkg/tbtc/coordination.go b/pkg/tbtc/coordination.go index 2dd75e9614..8eeb63e53a 100644 --- a/pkg/tbtc/coordination.go +++ b/pkg/tbtc/coordination.go @@ -591,9 +591,21 @@ func (ce *coordinationExecutor) getActionsChecklist( var actions []WalletActionType - // Redemption action is a priority action and should be checked on every - // coordination window. + // Redemption, reservation anchor, and reservation reanchor are priority + // actions and should be checked on every coordination window: like + // Redemption, they are custody-critical (an unaccepted reservation or a + // stale re-anchor risks reservation stranding, not just throughput), not + // throughput-heavy scans like the sweep/moving-funds actions gated below. + // + // A node that has not enabled the reservation feature + // (config.Reservations.Enabled=false) never registers a matching + // ProposalTask for these action types; pkg/tbtcpg.ProposalGenerator. + // Generate already treats a checklist action with no registered task as + // "unsupported" and skips it, so listing these unconditionally here is + // safe on non-reservation deployments. actions = append(actions, ActionRedemption) + actions = append(actions, ActionReservationAnchor) + actions = append(actions, ActionReservationReanchor) // Other actions should be checked with a lower frequency. The default // frequency is every 4 coordination windows. diff --git a/pkg/tbtc/coordination_test.go b/pkg/tbtc/coordination_test.go index c597048fb0..1b8671b417 100644 --- a/pkg/tbtc/coordination_test.go +++ b/pkg/tbtc/coordination_test.go @@ -546,7 +546,7 @@ func TestCoordinationExecutor_GetActionsChecklist(t *testing.T) { // Non-4th-window: only Redemption. "block 900": { coordinationBlock: 900, - expectedChecklist: []WalletActionType{ActionRedemption}, + expectedChecklist: []WalletActionType{ActionRedemption, ActionReservationAnchor, ActionReservationReanchor}, }, // Incorrect coordination window (windowIndex == 0, returns nil). "block 901": { @@ -556,11 +556,11 @@ func TestCoordinationExecutor_GetActionsChecklist(t *testing.T) { // Non-4th-window: only Redemption. "block 1800": { coordinationBlock: 1800, - expectedChecklist: []WalletActionType{ActionRedemption}, + expectedChecklist: []WalletActionType{ActionRedemption, ActionReservationAnchor, ActionReservationReanchor}, }, "block 2700": { coordinationBlock: 2700, - expectedChecklist: []WalletActionType{ActionRedemption}, + expectedChecklist: []WalletActionType{ActionRedemption, ActionReservationAnchor, ActionReservationReanchor}, }, // 4th-window (window 4): all actions present. Heartbeat randomly // selected for this specific seed. @@ -568,6 +568,8 @@ func TestCoordinationExecutor_GetActionsChecklist(t *testing.T) { coordinationBlock: 3600, expectedChecklist: []WalletActionType{ ActionRedemption, + ActionReservationAnchor, + ActionReservationReanchor, ActionDepositSweep, ActionMovedFundsSweep, ActionMovingFunds, @@ -576,21 +578,23 @@ func TestCoordinationExecutor_GetActionsChecklist(t *testing.T) { }, "block 4500": { coordinationBlock: 4500, - expectedChecklist: []WalletActionType{ActionRedemption}, + expectedChecklist: []WalletActionType{ActionRedemption, ActionReservationAnchor, ActionReservationReanchor}, }, "block 5400": { coordinationBlock: 5400, - expectedChecklist: []WalletActionType{ActionRedemption}, + expectedChecklist: []WalletActionType{ActionRedemption, ActionReservationAnchor, ActionReservationReanchor}, }, "block 6300": { coordinationBlock: 6300, - expectedChecklist: []WalletActionType{ActionRedemption}, + expectedChecklist: []WalletActionType{ActionRedemption, ActionReservationAnchor, ActionReservationReanchor}, }, // 4th-window (window 8): all actions present except heartbeat. "block 7200": { coordinationBlock: 7200, expectedChecklist: []WalletActionType{ ActionRedemption, + ActionReservationAnchor, + ActionReservationReanchor, ActionDepositSweep, ActionMovedFundsSweep, ActionMovingFunds, @@ -598,21 +602,23 @@ func TestCoordinationExecutor_GetActionsChecklist(t *testing.T) { }, "block 8100": { coordinationBlock: 8100, - expectedChecklist: []WalletActionType{ActionRedemption}, + expectedChecklist: []WalletActionType{ActionRedemption, ActionReservationAnchor, ActionReservationReanchor}, }, "block 9000": { coordinationBlock: 9000, - expectedChecklist: []WalletActionType{ActionRedemption}, + expectedChecklist: []WalletActionType{ActionRedemption, ActionReservationAnchor, ActionReservationReanchor}, }, "block 9900": { coordinationBlock: 9900, - expectedChecklist: []WalletActionType{ActionRedemption}, + expectedChecklist: []WalletActionType{ActionRedemption, ActionReservationAnchor, ActionReservationReanchor}, }, // 4th-window (window 12): all actions present except heartbeat. "block 10800": { coordinationBlock: 10800, expectedChecklist: []WalletActionType{ ActionRedemption, + ActionReservationAnchor, + ActionReservationReanchor, ActionDepositSweep, ActionMovedFundsSweep, ActionMovingFunds, @@ -620,23 +626,27 @@ func TestCoordinationExecutor_GetActionsChecklist(t *testing.T) { }, "block 11700": { coordinationBlock: 11700, - expectedChecklist: []WalletActionType{ActionRedemption}, + expectedChecklist: []WalletActionType{ActionRedemption, ActionReservationAnchor, ActionReservationReanchor}, }, "block 12600": { coordinationBlock: 12600, expectedChecklist: []WalletActionType{ ActionRedemption, + ActionReservationAnchor, + ActionReservationReanchor, }, }, "block 13500": { coordinationBlock: 13500, - expectedChecklist: []WalletActionType{ActionRedemption}, + expectedChecklist: []WalletActionType{ActionRedemption, ActionReservationAnchor, ActionReservationReanchor}, }, // 4th-window (window 16): all actions present except heartbeat. "block 14400": { coordinationBlock: 14400, expectedChecklist: []WalletActionType{ ActionRedemption, + ActionReservationAnchor, + ActionReservationReanchor, ActionDepositSweep, ActionMovedFundsSweep, ActionMovingFunds, @@ -701,6 +711,8 @@ func TestCoordinationExecutor_GetActionsChecklist_PostActivation(t *testing.T) { coordinationBlock: 24560100, expectedChecklist: []WalletActionType{ ActionRedemption, + ActionReservationAnchor, + ActionReservationReanchor, ActionDepositSweep, ActionMovedFundsSweep, }, @@ -710,6 +722,8 @@ func TestCoordinationExecutor_GetActionsChecklist_PostActivation(t *testing.T) { coordinationBlock: 24561000, expectedChecklist: []WalletActionType{ ActionRedemption, + ActionReservationAnchor, + ActionReservationReanchor, ActionDepositSweep, ActionMovedFundsSweep, }, @@ -719,6 +733,8 @@ func TestCoordinationExecutor_GetActionsChecklist_PostActivation(t *testing.T) { coordinationBlock: 24561900, expectedChecklist: []WalletActionType{ ActionRedemption, + ActionReservationAnchor, + ActionReservationReanchor, ActionDepositSweep, ActionMovedFundsSweep, }, @@ -730,6 +746,8 @@ func TestCoordinationExecutor_GetActionsChecklist_PostActivation(t *testing.T) { coordinationBlock: 24562800, expectedChecklist: []WalletActionType{ ActionRedemption, + ActionReservationAnchor, + ActionReservationReanchor, ActionDepositSweep, ActionMovedFundsSweep, ActionMovingFunds, @@ -740,6 +758,8 @@ func TestCoordinationExecutor_GetActionsChecklist_PostActivation(t *testing.T) { coordinationBlock: 24563700, expectedChecklist: []WalletActionType{ ActionRedemption, + ActionReservationAnchor, + ActionReservationReanchor, ActionDepositSweep, ActionMovedFundsSweep, }, @@ -752,6 +772,8 @@ func TestCoordinationExecutor_GetActionsChecklist_PostActivation(t *testing.T) { coordinationBlock: 24579000, expectedChecklist: []WalletActionType{ ActionRedemption, + ActionReservationAnchor, + ActionReservationReanchor, ActionDepositSweep, ActionMovedFundsSweep, ActionHeartbeat, @@ -764,6 +786,8 @@ func TestCoordinationExecutor_GetActionsChecklist_PostActivation(t *testing.T) { coordinationBlock: 24588000, expectedChecklist: []WalletActionType{ ActionRedemption, + ActionReservationAnchor, + ActionReservationReanchor, ActionDepositSweep, ActionMovedFundsSweep, ActionMovingFunds, @@ -778,6 +802,8 @@ func TestCoordinationExecutor_GetActionsChecklist_PostActivation(t *testing.T) { coordinationBlock: 24566400, expectedChecklist: []WalletActionType{ ActionRedemption, + ActionReservationAnchor, + ActionReservationReanchor, ActionDepositSweep, ActionMovedFundsSweep, ActionMovingFunds, @@ -838,6 +864,57 @@ func TestCoordinationExecutor_GetActionsChecklist_PostActivation(t *testing.T) { } } +// TestCoordinationExecutor_GetActionsChecklist_ReservationActionsAlwaysPresent +// is a dedicated regression guard for the reservation-checklist wiring gap: +// pkg/tbtcpg.ProposalGenerator.Generate only ever runs a task whose +// ActionType appears in this checklist, so ReservationAcceptanceTask and +// ReservationReanchorTask were structurally unreachable in production until +// ActionReservationAnchor/ActionReservationReanchor were added here. Unlike +// DepositSweep/MovedFundsSweep/MovingFunds, these are never frequency-gated +// - like Redemption, they are checked on every non-zero-index window, +// across every activation state and window-index parity. +func TestCoordinationExecutor_GetActionsChecklist_ReservationActionsAlwaysPresent(t *testing.T) { + // Cover both pre- and post-activation code paths, and both 4th and + // non-4th windows, across several distinct block/seed combinations. + coordinationBlocks := []uint64{ + 900, // pre-activation, non-4th window + 3600, // pre-activation, 4th window + 24560100, // post-activation, non-4th window + 24562800, // post-activation, 4th window + } + + executor := &coordinationExecutor{} + + for _, coordinationBlock := range coordinationBlocks { + window := newCoordinationWindow(coordinationBlock) + seed := sha256.Sum256( + big.NewInt(int64(window.coordinationBlock) + 2).Bytes(), + ) + + checklist := executor.getActionsChecklist( + window.index(), + seed, + window.coordinationBlock, + ) + + if !slices.Contains(checklist, ActionReservationAnchor) { + t.Errorf( + "block %d: ActionReservationAnchor must be present "+ + "in every coordination window's checklist", + coordinationBlock, + ) + } + + if !slices.Contains(checklist, ActionReservationReanchor) { + t.Errorf( + "block %d: ActionReservationReanchor must be present "+ + "in every coordination window's checklist", + coordinationBlock, + ) + } + } +} + // assertPostActivationSafety verifies the safety invariants that must hold // for every non-nil post-activation checklist: // - ActionRedemption is at index 0. @@ -883,9 +960,9 @@ func assertPostActivationSafety( } // assertChecklistOrdering verifies that actions appear in canonical priority -// order: Redemption < DepositSweep < MovedFundsSweep < MovingFunds < -// Heartbeat. Each consecutive pair of actions must have strictly increasing -// priority values. +// order: Redemption < ReservationAnchor < ReservationReanchor < DepositSweep +// < MovedFundsSweep < MovingFunds < Heartbeat. Each consecutive pair of +// actions must have strictly increasing priority values. func assertChecklistOrdering( t *testing.T, checklist []WalletActionType, @@ -893,11 +970,13 @@ func assertChecklistOrdering( t.Helper() actionPriority := map[WalletActionType]int{ - ActionRedemption: 0, - ActionDepositSweep: 1, - ActionMovedFundsSweep: 2, - ActionMovingFunds: 3, - ActionHeartbeat: 4, + ActionRedemption: 0, + ActionReservationAnchor: 1, + ActionReservationReanchor: 2, + ActionDepositSweep: 3, + ActionMovedFundsSweep: 4, + ActionMovingFunds: 5, + ActionHeartbeat: 6, } for i := 1; i < len(checklist); i++ { From cb4334627c85437594f03f29afd65fb278bb797b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Tue, 1 Sep 2026 16:05:20 +0000 Subject: [PATCH 3/9] test(tbtc): multi-signer simulated integration test for reservation coordination Implementation-plan.md Milestone 3, 'multi-signer simulated integration test' item (per user decision: build the test, leave the testnet-drill item as an agent-not-actionable tracked item since it needs live infra and calendar time, not code). Scales TestCoordinationExecutor_Coordinate's existing 3-operator harness - deterministic keypairs, real per-operator localChain fakes, a real shared netlocal.BroadcastChannel, one goroutine per operator running coordinationExecutor.coordinate concurrently - to ReservationAnchorProposal and ReservationReanchorProposal. This exercises the real leader/follower coordination round-trip (checklist generation -> leader election -> broadcast -> follower validation -> convergence) that no mocked pkg/tbtcpg unit test can cover, since those call task.Run(request) directly and never go through coordinationExecutor.coordinate. It also exercises PR #4277's protobuf marshaling of both proposal types over a real wire round-trip, since every follower unmarshals the leader's broadcast coordinationMessage. Depends on PR #4278 (this branch's parent): before that fix, ActionReservationAnchor/ActionReservationReanchor never appeared in getActionsChecklist's output, so every operator's checklist search in these tests would fall through to NoopProposal and fail - confirmed by temporarily reverting the checklist fix and re-running (both new tests failed with the expected NoopProposal mismatch), then restoring it. Found and fixed one bug in this test's own harness during verification: both new tests initially shared one netlocal broadcast channel name. getBroadcastChannel's registry is keyed by name and never releases old channels, so under -race (which changed goroutine/channel-delivery timing enough to surface it in ~every run), the reanchor test's follower sometimes received a stale broadcast left over from the anchor test's leader. Fixed by giving each test its own channel name; re-verified stable across 10 repeated -race runs plus the full non-race and race suites. Testing: - go test ./pkg/tbtc/...: 365/365 pass. - go test -race ./pkg/tbtc/...: clean, no data races, including -count=10 on just the two new tests. - go build ./... && go test ./...: full repo, 49 packages, zero FAIL. - gofmt -l / go vet: clean. --- pkg/tbtc/coordination_test.go | 414 ++++++++++++++++++++++++++++++++++ 1 file changed, 414 insertions(+) diff --git a/pkg/tbtc/coordination_test.go b/pkg/tbtc/coordination_test.go index 1b8671b417..dd4f249cd6 100644 --- a/pkg/tbtc/coordination_test.go +++ b/pkg/tbtc/coordination_test.go @@ -439,6 +439,420 @@ loop: ) } +// reservationCoordinationOperatorFixture bundles the per-operator state +// needed to run coordinationExecutor.coordinate as an independent +// in-process simulated node, sharing a local chain and broadcast channel +// with its peers the same way pkg/tbtc/node wires a real operator. +type reservationCoordinationOperatorFixture struct { + chain Chain + address chain.Address + channel net.BroadcastChannel + waitForBlockHeight func(ctx context.Context, blockHeight uint64) error +} + +// newReservationCoordinationOperator builds one simulated operator for the +// reservation multi-signer coordination tests below: a deterministic +// keypair (so leader election is reproducible across runs), a local chain +// fake wired to that keypair, and a broadcast channel joined to a local +// network shared by every operator in the same test so they exchange real +// coordinationMessage wire traffic - the same netlocal package +// TestCoordinationExecutor_Coordinate uses. channelName must be unique per +// test function: getBroadcastChannel's registry is keyed by name and never +// releases old channels, so two tests sharing a name can cross-deliver +// leftover broadcasts from one into the other's followers. +func newReservationCoordinationOperator( + t *testing.T, + privateKey int64, + coordinationBlock uint64, + channelName string, +) *reservationCoordinationOperatorFixture { + t.Helper() + + privateKeyBigInt := big.NewInt(privateKey) + x, y := local_v1.DefaultCurve.ScalarBaseMult(privateKeyBigInt.Bytes()) + + localChain := ConnectWithKey( + &operator.PrivateKey{ + PublicKey: operator.PublicKey{ + Curve: operator.Secp256k1, + X: x, + Y: y, + }, + D: privateKeyBigInt, + }, + 100*time.Millisecond, + ) + + localChain.setBlockHashByNumber( + coordinationBlock-32, + "1422996cbcbc38fc924a46f4df5f9064279d3ab43396e58386dac9b87440d64f", + ) + + operatorAddress, err := localChain.operatorAddress() + if err != nil { + t.Fatal(err) + } + + _, operatorPublicKey, err := localChain.OperatorKeyPair() + if err != nil { + t.Fatal(err) + } + + broadcastChannel, err := netlocal.ConnectWithKey(operatorPublicKey). + BroadcastChannelFor(channelName) + if err != nil { + t.Fatal(err) + } + + broadcastChannel.SetUnmarshaler(func() net.TaggedUnmarshaler { + return &coordinationMessage{} + }) + + waitForBlockHeight := func(ctx context.Context, blockHeight uint64) error { + blockCounter, err := localChain.BlockCounter() + if err != nil { + return err + } + + wait, err := blockCounter.BlockHeightWaiter(blockHeight) + if err != nil { + return err + } + + select { + case <-wait: + case <-ctx.Done(): + } + + return nil + } + + return &reservationCoordinationOperatorFixture{ + chain: localChain, + address: operatorAddress, + channel: broadcastChannel, + waitForBlockHeight: waitForBlockHeight, + } +} + +// reservationCoordinationReport captures one simulated operator's outcome +// from a single coordination round. +type reservationCoordinationReport struct { + operatorIndex int + result *coordinationResult + err error +} + +// runReservationCoordinationRound runs coordinationExecutor.coordinate +// concurrently for every given operator against the same window - one +// goroutine per operator, no shared mutable state beyond the local network +// fake - the same way pkg/tbtc/node's real coordination layer drives each +// node's own executor. Returns each operator's result sorted by operator +// index for deterministic assertions. +func runReservationCoordinationRound( + t *testing.T, + operators []*reservationCoordinationOperatorFixture, + coordinatedWallet wallet, + proposalGenerator CoordinationProposalGenerator, + membershipValidator *group.MembershipValidator, + protocolLatch *generator.ProtocolLatch, + window *coordinationWindow, +) []*reservationCoordinationReport { + t.Helper() + + reportChan := make(chan *reservationCoordinationReport, len(operators)) + + for i, currentOperator := range operators { + go func(operatorIndex int, op *reservationCoordinationOperatorFixture) { + executor := newCoordinationExecutor( + op.chain, + coordinatedWallet, + coordinatedWallet.membersByOperator(op.address), + op.address, + proposalGenerator, + op.channel, + membershipValidator, + protocolLatch, + op.waitForBlockHeight, + ) + + result, err := executor.coordinate(window) + + reportChan <- &reservationCoordinationReport{ + operatorIndex: operatorIndex, + result: result, + err: err, + } + }(i+1, currentOperator) + } + + reports := make([]*reservationCoordinationReport, 0, len(operators)) + for len(reports) < len(operators) { + reports = append(reports, <-reportChan) + } + + slices.SortFunc(reports, func(a, b *reservationCoordinationReport) int { + return a.operatorIndex - b.operatorIndex + }) + + return reports +} + +// newReservationCoordinationWallet returns the 3-operator wallet fixture +// shared by TestCoordinationExecutor_Coordinate_ReservationAnchor and +// TestCoordinationExecutor_Coordinate_ReservationReanchor: same wallet +// public key hash and operator-to-member-index layout as +// TestCoordinationExecutor_Coordinate, so leader election (operator2 wins +// at coordination block 900) is proven identical to that already-passing +// test rather than asserted freshly here. +func newReservationCoordinationWallet( + t *testing.T, + operators []*reservationCoordinationOperatorFixture, +) (wallet, [20]byte) { + t.Helper() + + // Uncompressed public key corresponding to the 20-byte public key hash: + // aa768412ceed10bd423c025542ca90071f9fb62d. + publicKeyHex, err := hex.DecodeString( + "0471e30bca60f6548d7b42582a478ea37ada63b402af7b3ddd57f0c95bb6843175" + + "aa0d2053a91a050a6797d85c38f2909cb7027f2344a01986aa2f9f8ca7a0c289", + ) + if err != nil { + t.Fatal(err) + } + + buffer, err := hex.DecodeString("aa768412ceed10bd423c025542ca90071f9fb62d") + if err != nil { + t.Fatal(err) + } + var publicKeyHash [20]byte + copy(publicKeyHash[:], buffer) + + operator1, operator2, operator3 := operators[0], operators[1], operators[2] + + coordinatedWallet := wallet{ + publicKey: mustUnmarshalPublicKey(t, publicKeyHex), + signingGroupOperators: []chain.Address{ + operator2.address, + operator3.address, + operator1.address, + operator1.address, + operator3.address, + operator2.address, + operator2.address, + operator3.address, + operator1.address, + operator1.address, + }, + } + + return coordinatedWallet, publicKeyHash +} + +// TestCoordinationExecutor_Coordinate_ReservationAnchor is the M1 +// acceptance-side leg of the Milestone 3 multi-signer simulated +// integration test: it scales TestCoordinationExecutor_Coordinate's +// 3-operator, real-broadcast-channel, real-leader-election harness to a +// ReservationAnchorProposal, proving the leader/follower coordination +// round-trip that no mocked unit test in pkg/tbtcpg (which calls +// task.Run(request) directly, never coordinationExecutor.coordinate) can +// cover. It also exercises PR #4277's protobuf marshaling of +// ReservationAnchorProposal over a real wire round-trip, since every +// follower unmarshals the leader's broadcast coordinationMessage. +// +// This test requires ActionReservationAnchor to actually appear in +// getActionsChecklist's output (fixed on this branch) - before that fix, +// every operator's checklist search below would fall through to +// NoopProposal and the assertion would fail. +func TestCoordinationExecutor_Coordinate_ReservationAnchor(t *testing.T) { + coordinationBlock := uint64(900) + + operator1 := newReservationCoordinationOperator(t, 1, coordinationBlock, "reservation-coordination-test-anchor") + operator2 := newReservationCoordinationOperator(t, 2, coordinationBlock, "reservation-coordination-test-anchor") + operator3 := newReservationCoordinationOperator(t, 3, coordinationBlock, "reservation-coordination-test-anchor") + operators := []*reservationCoordinationOperatorFixture{ + operator1, operator2, operator3, + } + + coordinatedWallet, publicKeyHash := newReservationCoordinationWallet(t, operators) + + expectedProposal := &ReservationAnchorProposal{ + DepositFundingTxHash: bitcoin.Hash{0x01, 0x02, 0x03}, + DepositFundingOutputIndex: 1, + RequestNonce: 7, + AnchorTxFee: big.NewInt(1500), + } + + proposalGenerator := newMockCoordinationProposalGenerator( + func( + walletPublicKeyHash [20]byte, + actionsChecklist []WalletActionType, + _ uint, + ) (CoordinationProposal, error) { + for _, action := range actionsChecklist { + if walletPublicKeyHash == publicKeyHash && action == ActionReservationAnchor { + return expectedProposal, nil + } + } + + return &NoopProposal{}, nil + }, + ) + + membershipValidator := group.NewMembershipValidator( + &testutils.MockLogger{}, + coordinatedWallet.signingGroupOperators, + Connect().Signing(), + ) + + protocolLatch := generator.NewProtocolLatch() + + window := newCoordinationWindow(coordinationBlock) + + reports := runReservationCoordinationRound( + t, + operators, + coordinatedWallet, + proposalGenerator, + membershipValidator, + protocolLatch, + window, + ) + + testutils.AssertIntsEqual(t, "reports count", 3, len(reports)) + + expectedResult := &coordinationResult{ + wallet: coordinatedWallet, + window: window, + leader: operator2.address, + proposal: expectedProposal, + faults: nil, + } + + for _, report := range reports { + if report.err != nil { + t.Fatalf( + "operator %d: unexpected error: %v", + report.operatorIndex, + report.err, + ) + } + if !reflect.DeepEqual(expectedResult, report.result) { + t.Errorf( + "operator %d: unexpected result\nexpected: %+v\nactual: %+v", + report.operatorIndex, + expectedResult, + report.result, + ) + } + } + + testutils.AssertBoolsEqual( + t, + "protocol latch state", + false, + protocolLatch.IsExecuting(), + ) +} + +// TestCoordinationExecutor_Coordinate_ReservationReanchor is the M1 +// re-anchor-side leg of the same Milestone 3 integration test: same +// 3-operator harness, wallet, and proven leader (operator2) as +// TestCoordinationExecutor_Coordinate_ReservationAnchor above - simulating +// the next coordination round in a reservation's lifecycle after its +// source wallet begins moving funds, this time converging on a +// ReservationReanchorProposal. +func TestCoordinationExecutor_Coordinate_ReservationReanchor(t *testing.T) { + coordinationBlock := uint64(900) + + operator1 := newReservationCoordinationOperator(t, 1, coordinationBlock, "reservation-coordination-test-reanchor") + operator2 := newReservationCoordinationOperator(t, 2, coordinationBlock, "reservation-coordination-test-reanchor") + operator3 := newReservationCoordinationOperator(t, 3, coordinationBlock, "reservation-coordination-test-reanchor") + operators := []*reservationCoordinationOperatorFixture{ + operator1, operator2, operator3, + } + + coordinatedWallet, publicKeyHash := newReservationCoordinationWallet(t, operators) + + expectedProposal := &ReservationReanchorProposal{ + ReservationKey: big.NewInt(424242), + RequestNonce: 4, + TargetWalletPublicKeyHash: [20]byte{0xf8, 0x7e, 0xb7}, + ReanchorTxFee: big.NewInt(1200), + } + + proposalGenerator := newMockCoordinationProposalGenerator( + func( + walletPublicKeyHash [20]byte, + actionsChecklist []WalletActionType, + _ uint, + ) (CoordinationProposal, error) { + for _, action := range actionsChecklist { + if walletPublicKeyHash == publicKeyHash && action == ActionReservationReanchor { + return expectedProposal, nil + } + } + + return &NoopProposal{}, nil + }, + ) + + membershipValidator := group.NewMembershipValidator( + &testutils.MockLogger{}, + coordinatedWallet.signingGroupOperators, + Connect().Signing(), + ) + + protocolLatch := generator.NewProtocolLatch() + + window := newCoordinationWindow(coordinationBlock) + + reports := runReservationCoordinationRound( + t, + operators, + coordinatedWallet, + proposalGenerator, + membershipValidator, + protocolLatch, + window, + ) + + testutils.AssertIntsEqual(t, "reports count", 3, len(reports)) + + expectedResult := &coordinationResult{ + wallet: coordinatedWallet, + window: window, + leader: operator2.address, + proposal: expectedProposal, + faults: nil, + } + + for _, report := range reports { + if report.err != nil { + t.Fatalf( + "operator %d: unexpected error: %v", + report.operatorIndex, + report.err, + ) + } + if !reflect.DeepEqual(expectedResult, report.result) { + t.Errorf( + "operator %d: unexpected result\nexpected: %+v\nactual: %+v", + report.operatorIndex, + expectedResult, + report.result, + ) + } + } + + testutils.AssertBoolsEqual( + t, + "protocol latch state", + false, + protocolLatch.IsExecuting(), + ) +} + func TestCoordinationExecutor_GetSeed(t *testing.T) { coordinationBlock := uint64(900) From bcb4725663153f231360a942041182843be063d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Tue, 1 Sep 2026 16:31:23 +0000 Subject: [PATCH 4/9] test(reservations): M2 test-coverage backfill (7 of 8 items) Implementation-plan.md Milestone 2, all items except the one item (ValidateReservationAnchorProposal/ValidateReservationReanchorProposal tests) that needs simulated-backend test infrastructure this repository doesn't have - see docs/spec/reservations/m1-keep-core- readiness/01-gap-analysis.md's new Minor row for that finding, and the item-8 dedup row for the golden-value equivalence decision. pkg/tbtc/reservation_test.go: - TestAssembleReservationAnchorTransaction: happy-path output shape (1-in-1-out, deposit value minus fee, P2WPKH to the target wallet). Also doubles as the golden reference for pkg/tbtcpg's dedup test. - TestAssembleReservationReanchorTransaction: same shape assertion for the re-anchor sibling. pkg/chain/ethereum/tbtc_test.go: - TestConvertReservationParametersFromAbiType: full 10-tuple field mapping, every field set to a distinct non-zero value so a swapped or dropped field can't hide behind a shared zero default. - TestConvertReservationFromAbiType_DropsCumulativeReanchorFee: pins the intentional CumulativeReanchorFee omission and verifies every other field maps correctly around it. pkg/tbtcpg/reservation_acceptance_test.go: - TestReservationAcceptanceTask_BoundaryChecks: 6-case table (at-limit accepts / one-over rejects) for MaxReservationsPerWallet, ReservationMinAmount, and ReservationMaxTotalAmount - BoundedLookback only ever used these as fixture data, never at the actual boundary. - TestReservationAcceptanceTask_ReservationParametersFetchedLive: runs the same task twice against the same deposit, mutating ReservationMinAmount between calls - proves ReservationParameters() is fetched live per call, not cached on the task. pkg/tbtcpg/reservation_anchor_dedup_test.go (new file, package tbtcpg - internal, not tbtcpg_test - to reach the unexported function): - TestBuildReservationAnchorTransaction_MatchesPkgTbtcGoldenOutput: buildReservationAnchorTransaction (pkg/tbtcpg) and assembleReservationAnchorTransaction (pkg/tbtc) are independently maintained copies of the same logic, both unexported in different packages - Go's visibility rules make a single test calling both impossible without a production-code change. This test and pkg/tbtc's TestAssembleReservationAnchorTransaction instead pin the identical golden input/output values (deposit 100000, fee 1500, output 98500) in each package, catching either copy drifting from the other without eliminating the underlying duplication (real fix deferred, per decision this session). Testing: - go test ./pkg/tbtc/... ./pkg/tbtcpg/... ./pkg/chain/ethereum/...: 519/519 pass. - go build ./... && go test ./...: full repo, 49 packages, zero FAIL. - gofmt -l / go vet: clean on all 4 changed/new files. --- pkg/chain/ethereum/tbtc_test.go | 118 ++++++++ pkg/tbtc/reservation_test.go | 207 ++++++++++++++ pkg/tbtcpg/reservation_acceptance_test.go | 289 ++++++++++++++++++++ pkg/tbtcpg/reservation_anchor_dedup_test.go | 159 +++++++++++ 4 files changed, 773 insertions(+) create mode 100644 pkg/tbtcpg/reservation_anchor_dedup_test.go diff --git a/pkg/chain/ethereum/tbtc_test.go b/pkg/chain/ethereum/tbtc_test.go index 1c9eef1be0..660893558d 100644 --- a/pkg/chain/ethereum/tbtc_test.go +++ b/pkg/chain/ethereum/tbtc_test.go @@ -16,8 +16,10 @@ import ( "github.com/ethereum/go-ethereum/common" "github.com/keep-network/keep-core/internal/testutils" + tbtcabi "github.com/keep-network/keep-core/pkg/chain/ethereum/tbtc/gen/abi" "github.com/keep-network/keep-core/pkg/chain/local_v1" "github.com/keep-network/keep-core/pkg/protocol/group" + "github.com/keep-network/keep-core/pkg/tbtc" ) func TestComputeOperatorsIDsHash(t *testing.T) { @@ -533,3 +535,119 @@ func TestBuildMovedFundsKey(t *testing.T) { movedFundsKey.Text(16), ) } + +// TestConvertReservationParametersFromAbiType verifies the full 10-tuple +// field mapping performed by convertReservationParametersFromAbiType. +// Gap-analysis Minor row: field count/order was not yet cross-checked +// against the live Solidity struct; every field below is set to a distinct +// non-zero value so a swapped or dropped field is caught, not masked by a +// shared zero-value default. +func TestConvertReservationParametersFromAbiType(t *testing.T) { + abiParameters := struct { + ReservationVault common.Address + ReservationMinAmount uint64 + ReservationTxMaxFee uint64 + ReservationTermSeconds uint32 + ReservationDissolutionDelay uint32 + ReservationMaxTotalAmount uint64 + ReservationTotalAmount uint64 + MaxReservationsPerWallet uint32 + ReservationActionTimeout uint32 + ReservationRenewalWindowSeconds uint32 + }{ + ReservationVault: common.HexToAddress("0x1111111111111111111111111111111111111a"), + ReservationMinAmount: 10000, + ReservationTxMaxFee: 20000, + ReservationTermSeconds: 30000, + ReservationDissolutionDelay: 40000, + ReservationMaxTotalAmount: 50000, + ReservationTotalAmount: 60000, + MaxReservationsPerWallet: 70000, + ReservationActionTimeout: 80000, + ReservationRenewalWindowSeconds: 90000, + } + + expected := &tbtc.ReservationParameters{ + ReservationVault: chain.Address(common.HexToAddress("0x1111111111111111111111111111111111111a").String()), + ReservationMinAmount: 10000, + ReservationTxMaxFee: 20000, + ReservationTermSeconds: 30000, + ReservationDissolutionDelay: 40000, + ReservationMaxTotalAmount: 50000, + ReservationTotalAmount: 60000, + MaxReservationsPerWallet: 70000, + ReservationActionTimeout: 80000, + ReservationRenewalWindowSeconds: 90000, + } + + actual := convertReservationParametersFromAbiType(abiParameters) + + if !reflect.DeepEqual(expected, actual) { + t.Errorf( + "unexpected reservation parameters\nexpected: [%+v]\nactual: [%+v]", + expected, + actual, + ) + } +} + +// TestConvertReservationFromAbiType_DropsCumulativeReanchorFee documents +// the intentional CumulativeReanchorFee drop performed by +// convertReservationFromAbiType: the field is written on-chain by every +// re-anchor hop but is not exposed on tbtc.Reservation because m1 has no +// fee-ceiling enforcement (own comment, tbtc.go:2672-2676). This test both +// pins that intentional omission and verifies every other field maps +// correctly - each field below is a distinct value so a future accidental +// restoration of CumulativeReanchorFee, or a swapped adjacent field, does +// not go unnoticed. +func TestConvertReservationFromAbiType_DropsCumulativeReanchorFee(t *testing.T) { + abiReservation := tbtcabi.ReservationReservationRequest{ + Owner: common.HexToAddress("0x1111111111111111111111111111111111111b"), + MintedAmount: 111, + AcceptedAt: 222, + WalletPubKeyHash: [20]byte{0x01, 0x02, 0x03}, + AnchorAmount: 333, + ExpiresAt: 444, + AnchorTxHash: [32]byte{0x04, 0x05, 0x06}, + AnchorTxOutputIndex: 555, + State: 1, // ReservationStateActive + RequestNonce: 666, + RetryCredit: true, + DissolutionEligibleAt: 777, + CumulativeReanchorFee: 888, // must not appear anywhere in the output + } + + expected := &tbtc.Reservation{ + Owner: chain.Address(common.HexToAddress("0x1111111111111111111111111111111111111b").String()), + MintedAmount: 111, + AcceptedAt: 222, + WalletPublicKeyHash: [20]byte{ + 0x01, 0x02, 0x03, + }, + AnchorUtxo: &bitcoin.UnspentTransactionOutput{ + Outpoint: &bitcoin.TransactionOutpoint{ + TransactionHash: bitcoin.Hash{0x04, 0x05, 0x06}, + OutputIndex: 555, + }, + Value: 333, + }, + ExpiresAt: 444, + State: tbtc.ReservationStateActive, + RequestNonce: 666, + RetryCredit: true, + DissolutionEligibleAt: 777, + } + + actual, err := convertReservationFromAbiType(abiReservation) + if err != nil { + t.Fatal(err) + } + + if !reflect.DeepEqual(expected, actual) { + t.Errorf( + "unexpected reservation\nexpected: [%+v]\nactual: [%+v]", + expected, + actual, + ) + } +} diff --git a/pkg/tbtc/reservation_test.go b/pkg/tbtc/reservation_test.go index 5deba71162..6da30df873 100644 --- a/pkg/tbtc/reservation_test.go +++ b/pkg/tbtc/reservation_test.go @@ -3,13 +3,16 @@ package tbtc import ( "crypto/ecdsa" "crypto/rand" + "crypto/sha256" "math/big" "reflect" "testing" "google.golang.org/protobuf/proto" + "github.com/keep-network/keep-core/internal/testutils" "github.com/keep-network/keep-core/pkg/bitcoin" + "github.com/keep-network/keep-core/pkg/chain" "github.com/keep-network/keep-core/pkg/tbtc/gen/pb" ) @@ -827,3 +830,207 @@ func TestAssembleReservationTransactions_InputValidation(t *testing.T) { "wallet main UTXO does not match the dissolution action snapshot", ) } + +// TestAssembleReservationAnchorTransaction verifies the happy-path output +// shape of assembleReservationAnchorTransaction: a 1-input-1-output +// transaction spending the reserved deposit's P2WSH UTXO into a single +// P2WPKH output controlled by the target wallet, valued at the deposit +// amount less the transaction fee. Gap-analysis Minor row: the only +// existing coverage (TestAssembleReservationTransactions_InputValidation) +// exercises the nil-deposit error path only. +// +// This test's deposit value (100000), fee (1500), and expected output +// value (98500) are the golden reference pkg/tbtcpg's +// TestBuildReservationAnchorTransaction_MatchesPkgTbtcGoldenOutput pins +// its independently-maintained duplicate of this logic +// (buildReservationAnchorTransaction, reservation_acceptance.go:579-585) +// against - the two functions live in different packages and are both +// unexported, so Go's visibility rules rule out a single test calling +// both directly; matching this golden value in each package's own test +// is the fallback that still catches either copy drifting from the +// other. +func TestAssembleReservationAnchorTransaction(t *testing.T) { + bitcoinChain := newLocalBitcoinChain() + + privateKeyValue := big.NewInt(100) + testWallet := generateWallet(privateKeyValue) + walletPublicKeyHash := bitcoin.PublicKeyHash(testWallet.publicKey) + walletScript, err := bitcoin.PayToWitnessPublicKeyHash(walletPublicKeyHash) + if err != nil { + t.Fatal(err) + } + + deposit := &Deposit{ + Depositor: chain.Address("0x1111111111111111111111111111111111111111"), + BlindingFactor: [8]byte{0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}, + WalletPublicKeyHash: walletPublicKeyHash, + RefundPublicKeyHash: [20]byte{0x02}, + RefundLocktime: [4]byte{0x03, 0x04, 0x05, 0x06}, + } + + depositScript, err := deposit.Script() + if err != nil { + t.Fatal(err) + } + + depositScriptHash := sha256.Sum256(depositScript) + depositLockingScript, err := bitcoin.PayToWitnessScriptHash(depositScriptHash) + if err != nil { + t.Fatal(err) + } + + fundingTransaction := &bitcoin.Transaction{ + Version: 1, + Inputs: []*bitcoin.TransactionInput{ + { + Outpoint: &bitcoin.TransactionOutpoint{ + TransactionHash: bitcoin.Hash{0x09}, + OutputIndex: 0, + }, + Sequence: 0xffffffff, + }, + }, + Outputs: []*bitcoin.TransactionOutput{ + { + Value: 100000, + PublicKeyScript: depositLockingScript, + }, + }, + } + if err := bitcoinChain.BroadcastTransaction(fundingTransaction); err != nil { + t.Fatal(err) + } + + deposit.Utxo = &bitcoin.UnspentTransactionOutput{ + Outpoint: &bitcoin.TransactionOutpoint{ + TransactionHash: fundingTransaction.Hash(), + OutputIndex: 0, + }, + Value: 100000, + } + + builder, err := assembleReservationAnchorTransaction( + bitcoinChain, + deposit, + walletPublicKeyHash, + 1500, + ) + if err != nil { + t.Fatal(err) + } + + transaction := signReservationTransaction( + t, + builder, + testWallet.publicKey, + privateKeyValue, + ) + + expectedOutputs := []*bitcoin.TransactionOutput{ + { + Value: 98500, + PublicKeyScript: walletScript, + }, + } + + if !reflect.DeepEqual(expectedOutputs, transaction.Outputs) { + t.Errorf( + "unexpected outputs\nexpected: [%+v]\nactual: [%+v]", + expectedOutputs, + transaction.Outputs, + ) + } + + testutils.AssertIntsEqual(t, "inputs count", 1, len(transaction.Inputs)) +} + +// TestAssembleReservationReanchorTransaction verifies the happy-path output +// shape of assembleReservationReanchorTransaction: a 1-input-1-output +// transaction spending the reservation's anchor UTXO into a single P2WPKH +// output controlled by the target wallet, valued at the anchor amount less +// the transaction fee. Gap-analysis Minor row: the only existing coverage +// (TestAssembleReservationTransactions_InputValidation) exercises the +// nil-anchor-UTXO error path only. +func TestAssembleReservationReanchorTransaction(t *testing.T) { + bitcoinChain := newLocalBitcoinChain() + + privateKeyValue := big.NewInt(100) + testWallet := generateWallet(privateKeyValue) + sourceWalletPublicKeyHash := bitcoin.PublicKeyHash(testWallet.publicKey) + sourceWalletScript, err := bitcoin.PayToWitnessPublicKeyHash(sourceWalletPublicKeyHash) + if err != nil { + t.Fatal(err) + } + + targetPrivateKeyValue := big.NewInt(200) + targetWallet := generateWallet(targetPrivateKeyValue) + targetWalletPublicKeyHash := bitcoin.PublicKeyHash(targetWallet.publicKey) + targetWalletScript, err := bitcoin.PayToWitnessPublicKeyHash(targetWalletPublicKeyHash) + if err != nil { + t.Fatal(err) + } + + fundingTransaction := &bitcoin.Transaction{ + Version: 1, + Inputs: []*bitcoin.TransactionInput{ + { + Outpoint: &bitcoin.TransactionOutpoint{ + TransactionHash: bitcoin.Hash{0x0a}, + OutputIndex: 0, + }, + Sequence: 0xffffffff, + }, + }, + Outputs: []*bitcoin.TransactionOutput{ + { + Value: 100000, + PublicKeyScript: sourceWalletScript, + }, + }, + } + if err := bitcoinChain.BroadcastTransaction(fundingTransaction); err != nil { + t.Fatal(err) + } + + anchorUtxo := &bitcoin.UnspentTransactionOutput{ + Outpoint: &bitcoin.TransactionOutpoint{ + TransactionHash: fundingTransaction.Hash(), + OutputIndex: 0, + }, + Value: 100000, + } + + builder, err := assembleReservationReanchorTransaction( + bitcoinChain, + anchorUtxo, + targetWalletPublicKeyHash, + 1500, + ) + if err != nil { + t.Fatal(err) + } + + transaction := signReservationTransaction( + t, + builder, + testWallet.publicKey, + privateKeyValue, + ) + + expectedOutputs := []*bitcoin.TransactionOutput{ + { + Value: 98500, + PublicKeyScript: targetWalletScript, + }, + } + + if !reflect.DeepEqual(expectedOutputs, transaction.Outputs) { + t.Errorf( + "unexpected outputs\nexpected: [%+v]\nactual: [%+v]", + expectedOutputs, + transaction.Outputs, + ) + } + + testutils.AssertIntsEqual(t, "inputs count", 1, len(transaction.Inputs)) +} diff --git a/pkg/tbtcpg/reservation_acceptance_test.go b/pkg/tbtcpg/reservation_acceptance_test.go index d08a11e93e..2a6ce3592f 100644 --- a/pkg/tbtcpg/reservation_acceptance_test.go +++ b/pkg/tbtcpg/reservation_acceptance_test.go @@ -667,3 +667,292 @@ func TestReservationAcceptanceTask_DepositNotReserved(t *testing.T) { } var _ = fmt.Sprintf + +// TestReservationAcceptanceTask_ReservationParametersFetchedLive verifies +// that ReservationParameters() (reservation_acceptance.go:133) is fetched +// fresh on every findReservationAcceptanceCandidate call rather than +// cached on the task. Runs the same task instance against the same +// qualifying deposit twice: once with a ReservationMinAmount the deposit +// clears, once (after mutating the chain fake's parameters in place, no +// new task) with one it doesn't. If the parameters were cached from the +// first call, the second run would still see the old, clearing value and +// wrongly accept. +func TestReservationAcceptanceTask_ReservationParametersFetchedLive(t *testing.T) { + ralc := newReservationAcceptanceLocalChain() + btcChain := tbtcpg.NewLocalBitcoinChain() + + walletPublicKeyHash := hexToByte20( + "8db50eb52063ea9d98b3eac91489a90f738986f6", + ) + + ralc.reservationParameters = &tbtc.ReservationParameters{ + ReservationVault: chain.Address( + "0xReservationVaultAddress1234567890abcdef12345678", + ), + ReservationMinAmount: 1000, + ReservationTxMaxFee: 5000, + MaxReservationsPerWallet: 5, + } + ralc.maxPerWalletAmount = 5000000 + ralc.maxSingleAmount = 5000000 + ralc.maxActive = 100 + + ralc.SetDepositMinAge(3600) + ralc.SetWallet( + walletPublicKeyHash, + &tbtc.WalletChainData{State: tbtc.StateLive}, + ) + + blockCounter := tbtcpg.NewMockBlockCounter() + blockCounter.SetCurrentBlock(300000) + ralc.SetBlockCounter(blockCounter) + + fundingTxHash := hashFromString( + "3333333333333333333333333333333333333333333333333333333333333333", + ) + dummyTx := &bitcoin.Transaction{ + Outputs: []*bitcoin.TransactionOutput{{ + Value: 0, + PublicKeyScript: append([]byte{0x00, 0x20}, make([]byte, 32)...), + }}, + } + btcChain.SetTransaction(fundingTxHash, dummyTx) + btcChain.SetTransactionConfirmations( + fundingTxHash, + tbtc.DepositSweepRequiredFundingTxConfirmations, + ) + ralc.SetDepositRequest( + fundingTxHash, + 0, + &tbtc.DepositChainRequest{ + Depositor: chain.Address("934b98637ca318a4d6e7ca6ffd1690b8e77df637"), + Amount: 2000000, + RevealedAt: time.Now().Add(-2 * time.Hour), + SweptAt: time.Unix(0, 0), + Vault: &[]chain.Address{chain.Address( + "0xReservationVaultAddress1234567890abcdef12345678", + )}[0], + }, + ) + depositKey := ralc.BuildDepositKey(fundingTxHash, 0) + ralc.reservedDeposits[depositKey.Text(16)] = true + + if err := ralc.AddPastDepositRevealedEvent( + &tbtc.DepositRevealedEventFilter{ + StartBlock: 300000 - tbtcpg.ReservationAcceptanceLookBackBlocks, + WalletPublicKeyHash: [][20]byte{walletPublicKeyHash}, + }, + &tbtc.DepositRevealedEvent{ + BlockNumber: 200000, + WalletPublicKeyHash: walletPublicKeyHash, + FundingTxHash: fundingTxHash, + FundingOutputIndex: 0, + }, + ); err != nil { + t.Fatal(err) + } + + task := tbtcpg.NewReservationAcceptanceTask(ralc, btcChain) + + request := &tbtc.CoordinationProposalRequest{ + WalletPublicKeyHash: walletPublicKeyHash, + } + + // First run: min amount (1000) is well below the deposit (2000000) - + // must accept. + _, shouldExecute, err := task.Run(request) + if err != nil { + t.Fatalf("unexpected error on first run: [%v]", err) + } + if !shouldExecute { + t.Fatalf("expected shouldExecute=true on first run, got false") + } + + // Mutate the chain fake's parameters in place - same task instance, + // same deposit, no new task created - then raise the min amount above + // the deposit's value. + ralc.reservationParameters.ReservationMinAmount = 3000000 + + // Second run: if ReservationParameters() were cached from the first + // run, this would still see ReservationMinAmount=1000 and wrongly + // accept again. + _, shouldExecute, err = task.Run(request) + if err != nil { + t.Fatalf("unexpected error on second run: [%v]", err) + } + if shouldExecute { + t.Fatalf( + "expected shouldExecute=false on second run after raising " + + "ReservationMinAmount above the deposit's value - a " + + "true result here means ReservationParameters() was " + + "cached from the first run instead of fetched live", + ) + } +} + +// TestReservationAcceptanceTask_BoundaryChecks exercises explicit +// at-limit/one-over-limit boundary crossings for the three eligibility +// caps in checkReservationAcceptanceEligibility +// (reservation_acceptance.go:424, :443, :475-478): +// MaxReservationsPerWallet, ReservationMinAmount, and +// ReservationMaxTotalAmount. TestReservationAcceptanceTask_BoundedLookback +// exercises these fields only as fixture data, never at their boundary +// value. +func TestReservationAcceptanceTask_BoundaryChecks(t *testing.T) { + tests := map[string]struct { + depositAmount uint64 + maxReservationsPerWallet uint32 + walletReservationsCount uint32 + reservationMinAmount uint64 + reservationMaxTotal uint64 + reservationTotal uint64 + expectAccept bool + }{ + "MaxReservationsPerWallet: below limit accepts": { + depositAmount: 2000000, + maxReservationsPerWallet: 5, + walletReservationsCount: 4, + reservationMinAmount: 1000, + expectAccept: true, + }, + "MaxReservationsPerWallet: at limit rejects": { + depositAmount: 2000000, + maxReservationsPerWallet: 5, + walletReservationsCount: 5, + reservationMinAmount: 1000, + expectAccept: false, + }, + "ReservationMinAmount: exactly at minimum accepts": { + depositAmount: 100000, + maxReservationsPerWallet: 5, + reservationMinAmount: 100000, + expectAccept: true, + }, + "ReservationMinAmount: one below minimum rejects": { + depositAmount: 99999, + maxReservationsPerWallet: 5, + reservationMinAmount: 100000, + expectAccept: false, + }, + "ReservationMaxTotalAmount: exactly at cap accepts": { + depositAmount: 2000000, + maxReservationsPerWallet: 5, + reservationMinAmount: 1000, + reservationTotal: 3000000, + reservationMaxTotal: 5000000, + expectAccept: true, + }, + "ReservationMaxTotalAmount: one over cap rejects": { + depositAmount: 2000000, + maxReservationsPerWallet: 5, + reservationMinAmount: 1000, + reservationTotal: 3000001, + reservationMaxTotal: 5000000, + expectAccept: false, + }, + } + + for testName, test := range tests { + t.Run(testName, func(t *testing.T) { + ralc := newReservationAcceptanceLocalChain() + btcChain := tbtcpg.NewLocalBitcoinChain() + + walletPublicKeyHash := hexToByte20( + "8db50eb52063ea9d98b3eac91489a90f738986f6", + ) + + ralc.reservationParameters = &tbtc.ReservationParameters{ + ReservationVault: chain.Address( + "0xReservationVaultAddress1234567890abcdef12345678", + ), + ReservationMinAmount: test.reservationMinAmount, + ReservationTxMaxFee: 5000, + MaxReservationsPerWallet: test.maxReservationsPerWallet, + ReservationMaxTotalAmount: test.reservationMaxTotal, + ReservationTotalAmount: test.reservationTotal, + } + ralc.maxPerWalletAmount = 50000000 + ralc.maxSingleAmount = 50000000 + ralc.maxActive = 100 + ralc.walletReservationsCount = test.walletReservationsCount + + ralc.SetDepositMinAge(3600) + ralc.SetWallet( + walletPublicKeyHash, + &tbtc.WalletChainData{State: tbtc.StateLive}, + ) + + currentBlock := uint64(300000) + blockCounter := tbtcpg.NewMockBlockCounter() + blockCounter.SetCurrentBlock(currentBlock) + ralc.SetBlockCounter(blockCounter) + + // Each subtest gets its own fresh ralc/btcChain instance + // (not a shared package-level registry), so a fixed hash is + // safe to reuse across cases. + fundingTxHash := hashFromString( + "4444444444444444444444444444444444444444444444444444444444444444", + ) + + dummyTx := &bitcoin.Transaction{ + Outputs: []*bitcoin.TransactionOutput{{ + Value: 0, + PublicKeyScript: append([]byte{0x00, 0x20}, make([]byte, 32)...), + }}, + } + btcChain.SetTransaction(fundingTxHash, dummyTx) + btcChain.SetTransactionConfirmations( + fundingTxHash, + tbtc.DepositSweepRequiredFundingTxConfirmations, + ) + ralc.SetDepositRequest( + fundingTxHash, + 0, + &tbtc.DepositChainRequest{ + Depositor: chain.Address("934b98637ca318a4d6e7ca6ffd1690b8e77df637"), + Amount: test.depositAmount, + RevealedAt: time.Now().Add(-2 * time.Hour), + SweptAt: time.Unix(0, 0), + Vault: &[]chain.Address{chain.Address( + "0xReservationVaultAddress1234567890abcdef12345678", + )}[0], + }, + ) + depositKey := ralc.BuildDepositKey(fundingTxHash, 0) + ralc.reservedDeposits[depositKey.Text(16)] = true + + if err := ralc.AddPastDepositRevealedEvent( + &tbtc.DepositRevealedEventFilter{ + StartBlock: currentBlock - tbtcpg.ReservationAcceptanceLookBackBlocks, + WalletPublicKeyHash: [][20]byte{walletPublicKeyHash}, + }, + &tbtc.DepositRevealedEvent{ + BlockNumber: 200000, + WalletPublicKeyHash: walletPublicKeyHash, + FundingTxHash: fundingTxHash, + FundingOutputIndex: 0, + }, + ); err != nil { + t.Fatal(err) + } + + task := tbtcpg.NewReservationAcceptanceTask(ralc, btcChain) + + request := &tbtc.CoordinationProposalRequest{ + WalletPublicKeyHash: walletPublicKeyHash, + } + + _, shouldExecute, err := task.Run(request) + if err != nil { + t.Fatalf("unexpected error: [%v]", err) + } + if shouldExecute != test.expectAccept { + t.Errorf( + "expected shouldExecute=%v, got %v", + test.expectAccept, + shouldExecute, + ) + } + }) + } +} diff --git a/pkg/tbtcpg/reservation_anchor_dedup_test.go b/pkg/tbtcpg/reservation_anchor_dedup_test.go new file mode 100644 index 0000000000..5868b596d4 --- /dev/null +++ b/pkg/tbtcpg/reservation_anchor_dedup_test.go @@ -0,0 +1,159 @@ +package tbtcpg + +import ( + "crypto/ecdsa" + "crypto/rand" + "crypto/sha256" + "math/big" + "reflect" + "testing" + + "github.com/keep-network/keep-core/pkg/bitcoin" + "github.com/keep-network/keep-core/pkg/chain" + "github.com/keep-network/keep-core/pkg/tbtc" + "github.com/keep-network/keep-core/pkg/tecdsa" +) + +// TestBuildReservationAnchorTransaction_MatchesPkgTbtcGoldenOutput is a +// golden-value equivalence check for the reservation-anchor-assembly +// duplication documented at reservation_acceptance.go:582-585: +// buildReservationAnchorTransaction here and +// pkg/tbtc.assembleReservationAnchorTransaction are two independently +// maintained copies of the same 1-input-1-output transaction-building +// logic ("Any change to the anchor transaction shape must be applied to +// both sites"). +// +// Both functions are unexported in different packages, so Go's visibility +// rules make a single test that calls both and diffs the result +// impossible without changing production code (exporting one, or +// extracting a shared helper - see the gap-analysis doc for that +// alternative). This test instead pins this package's output to a fixed, +// documented golden value; pkg/tbtc's +// TestAssembleReservationAnchorTransaction pins its sibling function to +// the identical deposit value (100000), fee (1500), and expected output +// value (98500) via a matching comment there. If either copy's shape +// diverges from the other, its own test starts failing against its half +// of this shared golden value - not as tight as calling both from one +// test, but it does catch drift in either direction. +func TestBuildReservationAnchorTransaction_MatchesPkgTbtcGoldenOutput(t *testing.T) { + bitcoinChain := NewLocalBitcoinChain() + + // Ad-hoc secp256k1 keypair used only to produce a technically valid + // ECDSA signature over the input's sighash (AddSignatures verifies the + // signature against the sighash, not against the deposit script's + // actual spending conditions - no Bitcoin VM execution happens in this + // unit test). The destination wallet public key hash below is an + // independent, arbitrary value; it does not need to correspond to + // this key. + privateKeyValue := big.NewInt(100) + x, y := tecdsa.Curve.ScalarBaseMult(privateKeyValue.Bytes()) + publicKey := &ecdsa.PublicKey{Curve: tecdsa.Curve, X: x, Y: y} + privateKey := &ecdsa.PrivateKey{PublicKey: *publicKey, D: privateKeyValue} + + walletPublicKeyHash := [20]byte{ + 0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff, 0x00, 0x11, 0x22, 0x33, + 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xaa, 0xbb, 0xcc, 0xdd, + } + walletScript, err := bitcoin.PayToWitnessPublicKeyHash(walletPublicKeyHash) + if err != nil { + t.Fatal(err) + } + + deposit := &tbtc.Deposit{ + Depositor: chain.Address("0x1111111111111111111111111111111111111111"), + BlindingFactor: [8]byte{0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}, + WalletPublicKeyHash: walletPublicKeyHash, + RefundPublicKeyHash: [20]byte{0x02}, + RefundLocktime: [4]byte{0x03, 0x04, 0x05, 0x06}, + } + + depositScript, err := deposit.Script() + if err != nil { + t.Fatal(err) + } + + depositScriptHash := sha256.Sum256(depositScript) + depositLockingScript, err := bitcoin.PayToWitnessScriptHash(depositScriptHash) + if err != nil { + t.Fatal(err) + } + + fundingTransaction := &bitcoin.Transaction{ + Version: 1, + Inputs: []*bitcoin.TransactionInput{ + { + Outpoint: &bitcoin.TransactionOutpoint{ + TransactionHash: bitcoin.Hash{0x0b}, + OutputIndex: 0, + }, + Sequence: 0xffffffff, + }, + }, + Outputs: []*bitcoin.TransactionOutput{ + { + Value: 100000, + PublicKeyScript: depositLockingScript, + }, + }, + } + bitcoinChain.SetTransaction(fundingTransaction.Hash(), fundingTransaction) + + deposit.Utxo = &bitcoin.UnspentTransactionOutput{ + Outpoint: &bitcoin.TransactionOutpoint{ + TransactionHash: fundingTransaction.Hash(), + OutputIndex: 0, + }, + Value: 100000, + } + + builder, err := buildReservationAnchorTransaction( + bitcoinChain, + deposit, + walletPublicKeyHash, + 1500, + ) + if err != nil { + t.Fatal(err) + } + + sigHashes, err := builder.ComputeSignatureHashes() + if err != nil { + t.Fatal(err) + } + + signatures := make([]*bitcoin.SignatureContainer, len(sigHashes)) + for i, sigHash := range sigHashes { + r, s, err := ecdsa.Sign(rand.Reader, privateKey, sigHash.Bytes()) + if err != nil { + t.Fatal(err) + } + signatures[i] = &bitcoin.SignatureContainer{ + R: r, + S: s, + PublicKey: publicKey, + } + } + + transaction, err := builder.AddSignatures(signatures) + if err != nil { + t.Fatal(err) + } + + // Golden value: deposit 100000, fee 1500 -> anchor output 98500. + // pkg/tbtc.TestAssembleReservationAnchorTransaction asserts the same + // arithmetic for its sibling function. + expectedOutputs := []*bitcoin.TransactionOutput{ + { + Value: 98500, + PublicKeyScript: walletScript, + }, + } + + if !reflect.DeepEqual(expectedOutputs, transaction.Outputs) { + t.Errorf( + "unexpected outputs\nexpected: [%+v]\nactual: [%+v]", + expectedOutputs, + transaction.Outputs, + ) + } +} From 5a645ab13bb2cff7d1d6433607bb240b0edce946 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Tue, 1 Sep 2026 16:37:42 +0000 Subject: [PATCH 5/9] fix(tbtc): regenerate message.pb.go with pinned protoc/protoc-gen-go The earlier regeneration used whatever protoc/protoc-gen-go happened to be installed (apt's protoc 3.21.12, go install's protoc-gen-go v1.30.0 at HEAD) - neither matches this file's own prior stamp (protoc-gen-go v1.28.0 / protoc v3.19.4) or any other .pb.go in the repo (every other generated file is v1.28.0 or v1.28.1 / protoc 3.7.1-3.21.5; this file was the only v1.30.0/3.21.12 outlier). Regenerated with protoc 3.19.4 (official release zip, not apt) and protoc-gen-go v1.28.0 (go install pinned to that version). Diff against the previous commit is exactly the two version-stamp lines - message content is otherwise byte-identical, confirming the earlier regeneration was semantically correct and this is a pure toolchain pin, not a functional change. --- pkg/tbtc/gen/pb/message.pb.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/tbtc/gen/pb/message.pb.go b/pkg/tbtc/gen/pb/message.pb.go index 41a7f348df..f728b35afe 100644 --- a/pkg/tbtc/gen/pb/message.pb.go +++ b/pkg/tbtc/gen/pb/message.pb.go @@ -1,7 +1,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: -// protoc-gen-go v1.30.0 -// protoc v3.21.12 +// protoc-gen-go v1.28.0 +// protoc v3.19.4 // source: pkg/tbtc/gen/pb/message.proto package pb From 61469875ff7827feda1a1235223d2e5957f986de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Thu, 3 Sep 2026 11:38:47 +0000 Subject: [PATCH 6/9] test(reservations): address multi-agent review findings Fixes 18 confirmed findings from the PR #4280 review (agent-docs/reviews/pr-4280/) plus corrections for merge-step data defects found while auditing the raw per-lens recall against the synthesized findings.json: pkg/tbtcpg/reservation_acceptance_test.go: - Fix tautological ReservationParametersFetchedLive test: ReservationParameters() now returns a deep copy so test-side mutation cannot corrupt the production candidate cache; add a no-mutation control subtest. - Add TestReservationAcceptanceTask_AnchorTransactionAssembly to lock the anchor-transaction wiring (output script/value) that was previously entirely untested. - Add TestReservationAcceptanceTask_GetReservationError pinning the current GetReservation-error fallthrough behavior. - Extend BoundaryChecks with a gross-passes/net-fails row and at-limit/ one-over coverage for the three previously untested eligibility gates (single-deposit, aggregate-wallet-amount, active-count caps). - Extract setupEligibleDeposit helper, removing 6x fixture duplication and inert reservedDeposits assignments. - Strengthen BoundedLookback to assert the specific drop path. - Correct doc-comment line citations and restore GetWalletError's comment. - Name the 710 sat fee constant with an accurate derivation comment. pkg/tbtc/reservation_test.go: - Fix TestAssembleReservationAnchorTransaction to use a distinct target wallet (was tautologically reusing the deposit's source wallet hash). - Correct docstrings overstating what prior tests did NOT cover; drop the dangling external gap-analysis doc references. - Note the tbtcpg-side reanchor-assembly coverage gap in the test docstring. pkg/chain/ethereum/tbtc_test.go: - Restore named vaultAddress variable (DRY regression). - Pad address literals to 40 hex digits (silent zero-left-pad bug). - Fix wrong doc-comment citation for the CumulativeReanchorFee omission rationale. - Replace tautological expected-value recomputation with literal constants. - Restore domain-meaningful fixture values in place of a meaningless arithmetic sequence. - Drop dangling external gap-analysis doc reference. go build ./..., go vet, gofmt, and go test ./... (49 packages) all clean. --- pkg/chain/ethereum/tbtc_test.go | 58 +- pkg/tbtc/reservation_test.go | 27 +- pkg/tbtcpg/reservation_acceptance_test.go | 847 ++++++++++++++++------ 3 files changed, 662 insertions(+), 270 deletions(-) diff --git a/pkg/chain/ethereum/tbtc_test.go b/pkg/chain/ethereum/tbtc_test.go index 5b04deb96f..01ff179797 100644 --- a/pkg/chain/ethereum/tbtc_test.go +++ b/pkg/chain/ethereum/tbtc_test.go @@ -750,11 +750,15 @@ func TestConvertReservationActionFromAbiType(t *testing.T) { // TestConvertReservationParametersFromAbiType verifies the full 10-tuple // field mapping performed by convertReservationParametersFromAbiType. -// Gap-analysis Minor row: field count/order was not yet cross-checked -// against the live Solidity struct; every field below is set to a distinct -// non-zero value so a swapped or dropped field is caught, not masked by a -// shared zero-value default. +// Field count/order had not previously been cross-checked against the +// live Solidity struct; every field below is set to a distinct non-zero +// value so a swapped or dropped field is caught, not masked by a shared +// zero-value default. func TestConvertReservationParametersFromAbiType(t *testing.T) { + vaultAddress := common.HexToAddress( + "0x111111111111111111111111111111111111111A", + ) + abiParameters := struct { ReservationVault common.Address ReservationMinAmount uint64 @@ -767,29 +771,29 @@ func TestConvertReservationParametersFromAbiType(t *testing.T) { ReservationActionTimeout uint32 ReservationRenewalWindowSeconds uint32 }{ - ReservationVault: common.HexToAddress("0x1111111111111111111111111111111111111a"), - ReservationMinAmount: 10000, - ReservationTxMaxFee: 20000, - ReservationTermSeconds: 30000, - ReservationDissolutionDelay: 40000, - ReservationMaxTotalAmount: 50000, - ReservationTotalAmount: 60000, - MaxReservationsPerWallet: 70000, - ReservationActionTimeout: 80000, - ReservationRenewalWindowSeconds: 90000, + ReservationVault: vaultAddress, + ReservationMinAmount: 1000, + ReservationTxMaxFee: 5000, + ReservationTermSeconds: 1209600, + ReservationDissolutionDelay: 3600, + ReservationMaxTotalAmount: 10000000, + ReservationTotalAmount: 2500000, + MaxReservationsPerWallet: 5, + ReservationActionTimeout: 86400, + ReservationRenewalWindowSeconds: 604800, } expected := &tbtc.ReservationParameters{ - ReservationVault: chain.Address(common.HexToAddress("0x1111111111111111111111111111111111111a").String()), - ReservationMinAmount: 10000, - ReservationTxMaxFee: 20000, - ReservationTermSeconds: 30000, - ReservationDissolutionDelay: 40000, - ReservationMaxTotalAmount: 50000, - ReservationTotalAmount: 60000, - MaxReservationsPerWallet: 70000, - ReservationActionTimeout: 80000, - ReservationRenewalWindowSeconds: 90000, + ReservationVault: chain.Address("0x111111111111111111111111111111111111111A"), + ReservationMinAmount: 1000, + ReservationTxMaxFee: 5000, + ReservationTermSeconds: 1209600, + ReservationDissolutionDelay: 3600, + ReservationMaxTotalAmount: 10000000, + ReservationTotalAmount: 2500000, + MaxReservationsPerWallet: 5, + ReservationActionTimeout: 86400, + ReservationRenewalWindowSeconds: 604800, } actual := convertReservationParametersFromAbiType(abiParameters) @@ -807,14 +811,14 @@ func TestConvertReservationParametersFromAbiType(t *testing.T) { // the intentional CumulativeReanchorFee drop performed by // convertReservationFromAbiType: the field is written on-chain by every // re-anchor hop but is not exposed on tbtc.Reservation because m1 has no -// fee-ceiling enforcement (own comment, tbtc.go:2672-2676). This test both +// fee-ceiling enforcement (own comment, tbtc.go:2637-2643). This test both // pins that intentional omission and verifies every other field maps // correctly - each field below is a distinct value so a future accidental // restoration of CumulativeReanchorFee, or a swapped adjacent field, does // not go unnoticed. func TestConvertReservationFromAbiType_DropsCumulativeReanchorFee(t *testing.T) { abiReservation := tbtcabi.ReservationReservationRequest{ - Owner: common.HexToAddress("0x1111111111111111111111111111111111111b"), + Owner: common.HexToAddress("0x111111111111111111111111111111111111111B"), MintedAmount: 111, AcceptedAt: 222, WalletPubKeyHash: [20]byte{0x01, 0x02, 0x03}, @@ -830,7 +834,7 @@ func TestConvertReservationFromAbiType_DropsCumulativeReanchorFee(t *testing.T) } expected := &tbtc.Reservation{ - Owner: chain.Address(common.HexToAddress("0x1111111111111111111111111111111111111b").String()), + Owner: chain.Address("0x111111111111111111111111111111111111111B"), MintedAmount: 111, AcceptedAt: 222, WalletPublicKeyHash: [20]byte{ diff --git a/pkg/tbtc/reservation_test.go b/pkg/tbtc/reservation_test.go index cf439fbd1e..048dbcd2dc 100644 --- a/pkg/tbtc/reservation_test.go +++ b/pkg/tbtc/reservation_test.go @@ -698,16 +698,22 @@ func TestReservationReanchorAction_Execute(t *testing.T) { // shape of AssembleReservationAnchorTransaction: a 1-input-1-output // transaction spending the reserved deposit's P2WSH UTXO into a single // P2WPKH output controlled by the target wallet, valued at the deposit -// amount less the transaction fee. Gap-analysis Minor row: the only -// existing coverage (TestAssembleReservationTransactions_InputValidation) -// exercises the nil-deposit error path only. +// amount less the transaction fee. Prior to this test, existing coverage +// (TestAssembleReservationTransactions_InputValidation, +// TestAssembleReservationTransactions_FeeBoundaries) exercised only +// validation-error and fee-boundary-error paths; no test asserted the +// happy-path output shape. func TestAssembleReservationAnchorTransaction(t *testing.T) { bitcoinChain := newLocalBitcoinChain() privateKeyValue := big.NewInt(100) testWallet := generateWallet(privateKeyValue) walletPublicKeyHash := bitcoin.PublicKeyHash(testWallet.publicKey) - walletScript, err := bitcoin.PayToWitnessPublicKeyHash(walletPublicKeyHash) + + targetPrivateKeyValue := big.NewInt(200) + targetWallet := generateWallet(targetPrivateKeyValue) + targetWalletPublicKeyHash := bitcoin.PublicKeyHash(targetWallet.publicKey) + targetWalletScript, err := bitcoin.PayToWitnessPublicKeyHash(targetWalletPublicKeyHash) if err != nil { t.Fatal(err) } @@ -764,7 +770,7 @@ func TestAssembleReservationAnchorTransaction(t *testing.T) { builder, err := AssembleReservationAnchorTransaction( bitcoinChain, deposit, - walletPublicKeyHash, + targetWalletPublicKeyHash, &ReservationAction{TxMaxFee: 1500}, 1500, ) @@ -782,7 +788,7 @@ func TestAssembleReservationAnchorTransaction(t *testing.T) { expectedOutputs := []*bitcoin.TransactionOutput{ { Value: 98500, - PublicKeyScript: walletScript, + PublicKeyScript: targetWalletScript, }, } @@ -801,9 +807,12 @@ func TestAssembleReservationAnchorTransaction(t *testing.T) { // shape of AssembleReservationReanchorTransaction: a 1-input-1-output // transaction spending the reservation's anchor UTXO into a single P2WPKH // output controlled by the target wallet, valued at the anchor amount less -// the transaction fee. Gap-analysis Minor row: the only existing coverage -// (TestAssembleReservationTransactions_InputValidation) exercises the -// nil-anchor-UTXO error path only. +// the transaction fee. Prior to this test, existing coverage +// (TestAssembleReservationTransactions_InputValidation, +// TestAssembleReservationTransactions_FeeBoundaries) exercised only +// validation-error and fee-boundary-error paths; no test asserted the +// happy-path output shape. Note that pkg/tbtcpg does not yet exercise the +// reanchor assembly path via this function. func TestAssembleReservationReanchorTransaction(t *testing.T) { bitcoinChain := newLocalBitcoinChain() diff --git a/pkg/tbtcpg/reservation_acceptance_test.go b/pkg/tbtcpg/reservation_acceptance_test.go index 2bfadcb5bf..2f9d75fd37 100644 --- a/pkg/tbtcpg/reservation_acceptance_test.go +++ b/pkg/tbtcpg/reservation_acceptance_test.go @@ -1,11 +1,15 @@ package tbtcpg_test import ( + "crypto/ecdsa" + "crypto/rand" + "crypto/sha256" "fmt" "math/big" "testing" "time" + "github.com/btcsuite/btcd/btcec" "github.com/ipfs/go-log/v2" "github.com/keep-network/keep-core/pkg/bitcoin" @@ -15,6 +19,13 @@ import ( "github.com/keep-network/keep-core/pkg/tbtcpg/internal/test" ) +// testAnchorFeeSat is the estimated reservation acceptance anchor fee in sats. +// It is computed as minWalletTxSatPerVByteFee (5 sat/vByte) multiplied by the +// estimated anchor transaction vsize (142 vBytes) because the test fixture's +// 1 sat/vByte fee rate oracle response is clamped to the 5 sat/vByte floor by +// applyWalletTxFeeFloor (see fee.go). +const testAnchorFeeSat = uint64(710) + // reservationAcceptanceLocalChain is a test-only mock of tbtcpg.Chain that // embeds the production LocalChain and adds reservation-specific behavior. // It exists as a separate type so this test file does not need to edit the @@ -33,6 +44,7 @@ type reservationAcceptanceLocalChain struct { reservedDeposits map[string]bool validateErr error getWalletErr error + getReservationErr error } func newReservationAcceptanceLocalChain() *reservationAcceptanceLocalChain { @@ -63,7 +75,11 @@ func (ralc *reservationAcceptanceLocalChain) ReservationParameters() ( *tbtc.ReservationParameters, error, ) { - return ralc.reservationParameters, nil + if ralc.reservationParameters == nil { + return nil, nil + } + paramsCopy := *ralc.reservationParameters + return ¶msCopy, nil } func (ralc *reservationAcceptanceLocalChain) ReservationCaps() ( @@ -119,6 +135,15 @@ func (ralc *reservationAcceptanceLocalChain) GetWallet( return ralc.LocalChain.GetWallet(walletPublicKeyHash) } +func (ralc *reservationAcceptanceLocalChain) GetReservation( + reservationKey *big.Int, +) (*tbtc.Reservation, error) { + if ralc.getReservationErr != nil { + return nil, ralc.getReservationErr + } + return ralc.LocalChain.GetReservation(reservationKey) +} + func (ralc *reservationAcceptanceLocalChain) ValidateReservationAnchorProposal( walletPublicKeyHash [20]byte, proposal *tbtc.ReservationAnchorProposal, @@ -284,10 +309,91 @@ func registerReservedDeposits( materialized.FundingTxHash, materialized.FundingOutputIndex, ) + // kept for parity with registerReservedDeposits; not read by ReservationAcceptanceTask ralc.reservedDeposits[depositKey.Text(16)] = true } } +// setupEligibleDeposit registers an eligible deposit funding transaction, +// deposit request, and matching DepositRevealedEvent on the mock chains. +// It returns the funding transaction hash. +func setupEligibleDeposit( + t *testing.T, + ralc *reservationAcceptanceLocalChain, + btcChain *tbtcpg.LocalBitcoinChain, + walletPublicKeyHash [20]byte, + currentBlock uint64, + depositAmount uint64, +) bitcoin.Hash { + t.Helper() + + fundingTxHash := hashFromString( + "2222222222222222222222222222222222222222222222222222222222222222", + ) + + dummyTx := &bitcoin.Transaction{ + Outputs: []*bitcoin.TransactionOutput{{ + Value: int64(depositAmount), + PublicKeyScript: append([]byte{0x00, 0x20}, make([]byte, 32)...), + }}, + } + btcChain.SetTransaction(fundingTxHash, dummyTx) + btcChain.SetEstimateSatPerVByteFee(1, 1) + btcChain.SetTransactionConfirmations( + fundingTxHash, + tbtc.DepositSweepRequiredFundingTxConfirmations, + ) + + vaultAddress := chain.Address( + "0xReservationVaultAddress1234567890abcdef12345678", + ) + if ralc.reservationParameters != nil && ralc.reservationParameters.ReservationVault != "" { + vaultAddress = ralc.reservationParameters.ReservationVault + } + + ralc.SetDepositRequest( + fundingTxHash, + 0, + &tbtc.DepositChainRequest{ + Depositor: chain.Address("934b98637ca318a4d6e7ca6ffd1690b8e77df637"), + Amount: depositAmount, + RevealedAt: time.Now().Add(-2 * time.Hour), + SweptAt: time.Unix(0, 0), + Vault: &vaultAddress, + }, + ) + + filterStartBlock := uint64(0) + if currentBlock > tbtcpg.ReservationAcceptanceLookBackBlocks { + filterStartBlock = currentBlock - tbtcpg.ReservationAcceptanceLookBackBlocks + } + + revealBlock := filterStartBlock + if revealBlock == 0 { + revealBlock = 1 + } + + err := ralc.AddPastDepositRevealedEvent( + &tbtc.DepositRevealedEventFilter{ + StartBlock: filterStartBlock, + EndBlock: ¤tBlock, + WalletPublicKeyHash: [][20]byte{walletPublicKeyHash}, + }, + &tbtc.DepositRevealedEvent{ + BlockNumber: revealBlock, + WalletPublicKeyHash: walletPublicKeyHash, + FundingTxHash: fundingTxHash, + FundingOutputIndex: 0, + Vault: &vaultAddress, + }, + ) + if err != nil { + t.Fatalf("failed to add past deposit revealed event: [%v]", err) + } + + return fundingTxHash +} + // expectedAnchorsEqual compares two proposal objects field-by-field. // deep.Equal cannot be used for this: by default it does not descend into // unexported fields, and *big.Int's representation is entirely unexported, @@ -430,6 +536,217 @@ func TestReservationAcceptanceTask_Run(t *testing.T) { } } +// TestReservationAcceptanceTask_AnchorTransactionAssembly verifies the +// wiring of AssembleReservationAnchorTransaction: it ensures that an assembled +// anchor transaction can be signed and produces a valid 1-input-1-output +// Bitcoin transaction paying the correct wallet P2WPKH output script with +// value equal to deposit amount minus the estimated anchor fee. +func TestReservationAcceptanceTask_AnchorTransactionAssembly(t *testing.T) { + ralc := newReservationAcceptanceLocalChain() + btcChain := tbtcpg.NewLocalBitcoinChain() + btcChain.SetEstimateSatPerVByteFee(1, 1) + + privateKey, err := ecdsa.GenerateKey(btcec.S256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + walletPublicKeyHash := bitcoin.PublicKeyHash(&privateKey.PublicKey) + + depositAmount := uint64(2000000) + currentBlock := uint64(300000) + + ralc.reservationParameters = &tbtc.ReservationParameters{ + ReservationVault: chain.Address( + "0xReservationVaultAddress1234567890abcdef12345678", + ), + ReservationMinAmount: 1000, + ReservationTxMaxFee: 5000, + MaxReservationsPerWallet: 5, + } + ralc.maxPerWalletAmount = 50000000 + ralc.maxSingleAmount = 50000000 + ralc.maxActive = 100 + + ralc.SetDepositMinAge(3600) + ralc.SetWallet( + walletPublicKeyHash, + &tbtc.WalletChainData{State: tbtc.StateLive}, + ) + + blockCounter := tbtcpg.NewMockBlockCounter() + blockCounter.SetCurrentBlock(currentBlock) + ralc.SetBlockCounter(blockCounter) + + deposit := &tbtc.Deposit{ + Depositor: chain.Address("934b98637ca318a4d6e7ca6ffd1690b8e77df637"), + BlindingFactor: [8]byte{0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08}, + WalletPublicKeyHash: walletPublicKeyHash, + RefundPublicKeyHash: [20]byte{0x02}, + RefundLocktime: [4]byte{0x03, 0x04, 0x05, 0x06}, + Vault: &[]chain.Address{chain.Address( + "0xReservationVaultAddress1234567890abcdef12345678", + )}[0], + } + + depositScript, err := deposit.Script() + if err != nil { + t.Fatal(err) + } + + depositScriptHash := sha256.Sum256(depositScript) + depositLockingScript, err := bitcoin.PayToWitnessScriptHash(depositScriptHash) + if err != nil { + t.Fatal(err) + } + + fundingTx := &bitcoin.Transaction{ + Version: 1, + Inputs: []*bitcoin.TransactionInput{ + { + Outpoint: &bitcoin.TransactionOutpoint{ + TransactionHash: bitcoin.Hash{0x09}, + OutputIndex: 0, + }, + Sequence: 0xffffffff, + }, + }, + Outputs: []*bitcoin.TransactionOutput{ + { + Value: int64(depositAmount), + PublicKeyScript: depositLockingScript, + }, + }, + } + fundingTxHash := fundingTx.Hash() + btcChain.SetTransaction(fundingTxHash, fundingTx) + btcChain.SetTransactionConfirmations( + fundingTxHash, + tbtc.DepositSweepRequiredFundingTxConfirmations, + ) + + deposit.Utxo = &bitcoin.UnspentTransactionOutput{ + Outpoint: &bitcoin.TransactionOutpoint{ + TransactionHash: fundingTxHash, + OutputIndex: 0, + }, + Value: int64(depositAmount), + } + + ralc.SetDepositRequest( + fundingTxHash, + 0, + &tbtc.DepositChainRequest{ + Depositor: deposit.Depositor, + Amount: depositAmount, + RevealedAt: time.Now().Add(-2 * time.Hour), + SweptAt: time.Unix(0, 0), + Vault: deposit.Vault, + }, + ) + + filterStartBlock := currentBlock - tbtcpg.ReservationAcceptanceLookBackBlocks + if err := ralc.AddPastDepositRevealedEvent( + &tbtc.DepositRevealedEventFilter{ + StartBlock: filterStartBlock, + EndBlock: ¤tBlock, + WalletPublicKeyHash: [][20]byte{walletPublicKeyHash}, + }, + &tbtc.DepositRevealedEvent{ + BlockNumber: 200000, + WalletPublicKeyHash: walletPublicKeyHash, + FundingTxHash: fundingTxHash, + FundingOutputIndex: 0, + Vault: deposit.Vault, + BlindingFactor: deposit.BlindingFactor, + RefundPublicKeyHash: deposit.RefundPublicKeyHash, + RefundLocktime: deposit.RefundLocktime, + }, + ); err != nil { + t.Fatal(err) + } + + task := tbtcpg.NewReservationAcceptanceTask(ralc, btcChain) + proposal, shouldExecute, err := task.Run(&tbtc.CoordinationProposalRequest{ + WalletPublicKeyHash: walletPublicKeyHash, + }) + if err != nil { + t.Fatalf("unexpected error running task: [%v]", err) + } + if !shouldExecute { + t.Fatalf("expected shouldExecute=true, got false") + } + if proposal == nil { + t.Fatalf("expected non-nil proposal") + } + + anchorProposal, ok := proposal.(*tbtc.ReservationAnchorProposal) + if !ok { + t.Fatalf("expected *ReservationAnchorProposal, got %T", proposal) + } + + // Re-assemble and sign to verify transaction builder output properties. + builder, err := tbtc.AssembleReservationAnchorTransaction( + btcChain, + deposit, + walletPublicKeyHash, + &tbtc.ReservationAction{TxMaxFee: 5000}, + anchorProposal.AnchorTxFee.Int64(), + ) + if err != nil { + t.Fatalf("failed to assemble reservation anchor transaction: [%v]", err) + } + + sigHashes, err := builder.ComputeSignatureHashes() + if err != nil { + t.Fatalf("failed to compute signature hashes: [%v]", err) + } + signatures := make([]*bitcoin.SignatureContainer, len(sigHashes)) + for i, sigHash := range sigHashes { + r, s, err := ecdsa.Sign(rand.Reader, privateKey, sigHash.Bytes()) + if err != nil { + t.Fatalf("failed to sign input: [%v]", err) + } + signatures[i] = &bitcoin.SignatureContainer{ + R: r, + S: s, + PublicKey: &privateKey.PublicKey, + } + } + + signedTx, err := builder.AddSignatures(signatures) + if err != nil { + t.Fatalf("failed to add signatures: [%v]", err) + } + + if len(signedTx.Inputs) != 1 { + t.Errorf("expected 1 input, got %d", len(signedTx.Inputs)) + } + if len(signedTx.Outputs) != 1 { + t.Errorf("expected 1 output, got %d", len(signedTx.Outputs)) + } + + expectedOutputScript, err := bitcoin.PayToWitnessPublicKeyHash(walletPublicKeyHash) + if err != nil { + t.Fatal(err) + } + expectedOutputValue := int64(depositAmount) - anchorProposal.AnchorTxFee.Int64() + + if signedTx.Outputs[0].Value != expectedOutputValue { + t.Errorf( + "unexpected output value\nexpected: [%d]\nactual: [%d]", + expectedOutputValue, + signedTx.Outputs[0].Value, + ) + } + if string(signedTx.Outputs[0].PublicKeyScript) != string(expectedOutputScript) { + t.Errorf( + "unexpected output script\nexpected: [%x]\nactual: [%x]", + expectedOutputScript, + signedTx.Outputs[0].PublicKeyScript, + ) + } +} + // TestReservationAcceptanceTask_NoCandidates verifies that the task is a // no-op when the chain has no reserved deposits. func TestReservationAcceptanceTask_NoCandidates(t *testing.T) { @@ -481,8 +798,6 @@ func TestReservationAcceptanceTask_NoCandidates(t *testing.T) { // look-back window is applied when the current block exceeds it. func TestReservationAcceptanceTask_BoundedLookback(t *testing.T) { currentBlock := uint64(400000) - expectedStartBlock := currentBlock - - tbtcpg.ReservationAcceptanceLookBackBlocks ralc := newReservationAcceptanceLocalChain() btcChain := tbtcpg.NewLocalBitcoinChain() @@ -513,7 +828,7 @@ func TestReservationAcceptanceTask_BoundedLookback(t *testing.T) { blockCounter.SetCurrentBlock(currentBlock) ralc.SetBlockCounter(blockCounter) - // Event below the look-back start block must NOT be returned. + // Register an event below the look-back start block (block 1). oldFundingTxHash := hashFromString( "1111111111111111111111111111111111111111111111111111111111111111", ) @@ -533,65 +848,38 @@ func TestReservationAcceptanceTask_BoundedLookback(t *testing.T) { t.Fatal(err) } - // Event at the look-back start block must be returned. Mark it as - // reserved and provide a deposit request. - fundingTxHash := hashFromString( - "2222222222222222222222222222222222222222222222222222222222222222", - ) - dummyTx := &bitcoin.Transaction{ - Outputs: []*bitcoin.TransactionOutput{{ - Value: 0, - PublicKeyScript: append([]byte{0x00, 0x20}, make([]byte, 32)...), - }}, - } - btcChain.SetTransaction(fundingTxHash, dummyTx) - btcChain.SetEstimateSatPerVByteFee(1, 1) - btcChain.SetTransactionConfirmations( - fundingTxHash, - tbtc.DepositSweepRequiredFundingTxConfirmations, - ) - ralc.SetDepositRequest( - fundingTxHash, - 0, - &tbtc.DepositChainRequest{ - Depositor: chain.Address("934b98637ca318a4d6e7ca6ffd1690b8e77df637"), - Amount: 2000000, - RevealedAt: time.Now().Add(-2 * time.Hour), - SweptAt: time.Unix(0, 0), - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], - }, - ) - depositKey := ralc.BuildDepositKey(fundingTxHash, 0) - ralc.reservedDeposits[depositKey.Text(16)] = true - - if err := ralc.AddPastDepositRevealedEvent( - &tbtc.DepositRevealedEventFilter{ - StartBlock: expectedStartBlock, - EndBlock: ¤tBlock, - WalletPublicKeyHash: [][20]byte{walletPublicKeyHash}, - }, - &tbtc.DepositRevealedEvent{ - BlockNumber: expectedStartBlock, - WalletPublicKeyHash: walletPublicKeyHash, - FundingTxHash: fundingTxHash, - FundingOutputIndex: 0, - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], - }, - ); err != nil { - t.Fatal(err) - } - task := tbtcpg.NewReservationAcceptanceTask(ralc, btcChain) - request := &tbtc.CoordinationProposalRequest{ WalletPublicKeyHash: walletPublicKeyHash, } + // First run: only the old deposit exists below the look-back window. + // Because the candidate satisfies all other conditions (valid wallet, + // clear caps), the shouldExecute=false result is strictly attributable + // to exclusion by the look-back start block filter. proposal, shouldExecute, err := task.Run(request) + if err != nil { + t.Fatalf("unexpected error on old deposit run: [%v]", err) + } + if shouldExecute { + t.Errorf("expected shouldExecute=false for deposit below lookback window, got true") + } + if proposal != nil { + t.Errorf("expected nil proposal for deposit below lookback window, got [%+v]", proposal) + } + + // Register an eligible deposit at the look-back start block. + fundingTxHash := setupEligibleDeposit( + t, + ralc, + btcChain, + walletPublicKeyHash, + currentBlock, + 2000000, + ) + + // Second run: the deposit at the look-back start block must be found and accepted. + proposal, shouldExecute, err = task.Run(request) if err != nil { t.Fatalf("unexpected error: [%v]", err) } @@ -701,6 +989,10 @@ func TestReservationAcceptanceTask_DepositNotReserved(t *testing.T) { } } +// TestReservationAcceptanceTask_GetWalletError exercises the GetWallet +// error passthrough inside checkReservationAcceptanceEligibility: a +// reserved deposit candidate is discovered and matches the reservation +// vault, but the candidate wallet's chain data fails to load. func TestReservationAcceptanceTask_GetWalletError(t *testing.T) { ralc := newReservationAcceptanceLocalChain() btcChain := tbtcpg.NewLocalBitcoinChain() @@ -731,46 +1023,14 @@ func TestReservationAcceptanceTask_GetWalletError(t *testing.T) { blockCounter.SetCurrentBlock(currentBlock) ralc.SetBlockCounter(blockCounter) - fundingTxHash := hashFromString( - "4444444444444444444444444444444444444444444444444444444444444444", - ) - btcChain.SetTransaction(fundingTxHash, &bitcoin.Transaction{}) - btcChain.SetTransactionConfirmations( - fundingTxHash, - tbtc.DepositSweepRequiredFundingTxConfirmations, - ) - ralc.SetDepositRequest( - fundingTxHash, - 0, - &tbtc.DepositChainRequest{ - Amount: 2000000, - RevealedAt: time.Now().Add(-2 * time.Hour), - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], - }, + setupEligibleDeposit( + t, + ralc, + btcChain, + walletPublicKeyHash, + currentBlock, + 2000000, ) - depositKey := ralc.BuildDepositKey(fundingTxHash, 0) - ralc.reservedDeposits[depositKey.Text(16)] = true - - if err := ralc.AddPastDepositRevealedEvent( - &tbtc.DepositRevealedEventFilter{ - StartBlock: 0, - EndBlock: ¤tBlock, - WalletPublicKeyHash: [][20]byte{walletPublicKeyHash}, - }, - &tbtc.DepositRevealedEvent{ - BlockNumber: 1, - WalletPublicKeyHash: walletPublicKeyHash, - FundingTxHash: fundingTxHash, - FundingOutputIndex: 0, - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], - }, - ); err != nil { - t.Fatal(err) - } task := tbtcpg.NewReservationAcceptanceTask(ralc, btcChain) @@ -788,10 +1048,12 @@ func TestReservationAcceptanceTask_GetWalletError(t *testing.T) { } } -func TestReservationAcceptanceTask_ReservationParametersFetchedLive(t *testing.T) { +// TestReservationAcceptanceTask_GetReservationError verifies that when +// GetReservation fails, the task logs the error and falls through to +// RequestReservationAcceptance, continuing with proposal emission. +func TestReservationAcceptanceTask_GetReservationError(t *testing.T) { ralc := newReservationAcceptanceLocalChain() btcChain := tbtcpg.NewLocalBitcoinChain() - btcChain.SetEstimateSatPerVByteFee(1, 1) walletPublicKeyHash := hexToByte20( "8db50eb52063ea9d98b3eac91489a90f738986f6", @@ -815,106 +1077,198 @@ func TestReservationAcceptanceTask_ReservationParametersFetchedLive(t *testing.T &tbtc.WalletChainData{State: tbtc.StateLive}, ) + currentBlock := uint64(300000) blockCounter := tbtcpg.NewMockBlockCounter() - blockCounter.SetCurrentBlock(300000) + blockCounter.SetCurrentBlock(currentBlock) ralc.SetBlockCounter(blockCounter) - fundingTxHash := hashFromString( - "3333333333333333333333333333333333333333333333333333333333333333", - ) - dummyTx := &bitcoin.Transaction{ - Outputs: []*bitcoin.TransactionOutput{{ - Value: 0, - PublicKeyScript: append([]byte{0x00, 0x20}, make([]byte, 32)...), - }}, - } - btcChain.SetTransaction(fundingTxHash, dummyTx) - btcChain.SetTransactionConfirmations( - fundingTxHash, - tbtc.DepositSweepRequiredFundingTxConfirmations, - ) - ralc.SetDepositRequest( - fundingTxHash, - 0, - &tbtc.DepositChainRequest{ - Depositor: chain.Address("934b98637ca318a4d6e7ca6ffd1690b8e77df637"), - Amount: 2000000, - RevealedAt: time.Now().Add(-2 * time.Hour), - SweptAt: time.Unix(0, 0), - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], - }, + fundingTxHash := setupEligibleDeposit( + t, + ralc, + btcChain, + walletPublicKeyHash, + currentBlock, + 2000000, ) - depositKey := ralc.BuildDepositKey(fundingTxHash, 0) - ralc.reservedDeposits[depositKey.Text(16)] = true - currentBlock := uint64(300000) - if err := ralc.AddPastDepositRevealedEvent( - &tbtc.DepositRevealedEventFilter{ - StartBlock: 300000 - tbtcpg.ReservationAcceptanceLookBackBlocks, - EndBlock: ¤tBlock, - WalletPublicKeyHash: [][20]byte{walletPublicKeyHash}, - }, - &tbtc.DepositRevealedEvent{ - BlockNumber: 200000, - WalletPublicKeyHash: walletPublicKeyHash, - FundingTxHash: fundingTxHash, - FundingOutputIndex: 0, - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], - }, - ); err != nil { - t.Fatal(err) - } + // Force GetReservation to return an error. + ralc.getReservationErr = fmt.Errorf("simulated get reservation error") task := tbtcpg.NewReservationAcceptanceTask(ralc, btcChain) - request := &tbtc.CoordinationProposalRequest{ + proposal, shouldExecute, err := task.Run(&tbtc.CoordinationProposalRequest{ WalletPublicKeyHash: walletPublicKeyHash, - } - - // First run: min amount (1000) is well below the deposit (2000000) - - // must accept. - _, shouldExecute, err := task.Run(request) + }) if err != nil { - t.Fatalf("unexpected error on first run: [%v]", err) + t.Fatalf("unexpected error: [%v]", err) } if !shouldExecute { - t.Fatalf("expected shouldExecute=true on first run, got false") + t.Errorf("expected shouldExecute=true, got false") + } + if proposal == nil { + t.Fatalf("expected non-nil proposal") } - // Mutate the chain fake's parameters in place - same task instance, - // same deposit, no new task created - then raise the min amount above - // the deposit's value. - ralc.reservationParameters.ReservationMinAmount = 3000000 - - // Second run: if ReservationParameters() were cached from the first - // run, this would still see ReservationMinAmount=1000 and wrongly - // accept again. - _, shouldExecute, err = task.Run(request) - if err != nil { - t.Fatalf("unexpected error on second run: [%v]", err) + actualProposal, ok := proposal.(*tbtc.ReservationAnchorProposal) + if !ok { + t.Fatalf("expected *ReservationAnchorProposal, got %T", proposal) } - if shouldExecute { - t.Fatalf( - "expected shouldExecute=false on second run after raising " + - "ReservationMinAmount above the deposit's value - a " + - "true result here means ReservationParameters() was " + - "cached from the first run instead of fetched live", + if actualProposal.DepositFundingTxHash != fundingTxHash { + t.Errorf( + "unexpected deposit funding tx hash\nexpected: %s\nactual: %s", + fundingTxHash.Hex(bitcoin.ReversedByteOrder), + actualProposal.DepositFundingTxHash.Hex(bitcoin.ReversedByteOrder), ) } } +func TestReservationAcceptanceTask_ReservationParametersFetchedLive(t *testing.T) { + walletPublicKeyHash := hexToByte20( + "8db50eb52063ea9d98b3eac91489a90f738986f6", + ) + currentBlock := uint64(300000) + + t.Run("without parameter mutation accepts on subsequent run", func(t *testing.T) { + ralc := newReservationAcceptanceLocalChain() + btcChain := tbtcpg.NewLocalBitcoinChain() + + ralc.reservationParameters = &tbtc.ReservationParameters{ + ReservationVault: chain.Address( + "0xReservationVaultAddress1234567890abcdef12345678", + ), + ReservationMinAmount: 1000, + ReservationTxMaxFee: 5000, + MaxReservationsPerWallet: 5, + } + ralc.maxPerWalletAmount = 5000000 + ralc.maxSingleAmount = 5000000 + ralc.maxActive = 100 + + ralc.SetDepositMinAge(3600) + ralc.SetWallet( + walletPublicKeyHash, + &tbtc.WalletChainData{State: tbtc.StateLive}, + ) + + blockCounter := tbtcpg.NewMockBlockCounter() + blockCounter.SetCurrentBlock(currentBlock) + ralc.SetBlockCounter(blockCounter) + + setupEligibleDeposit( + t, + ralc, + btcChain, + walletPublicKeyHash, + currentBlock, + 2000000, + ) + + task := tbtcpg.NewReservationAcceptanceTask(ralc, btcChain) + request := &tbtc.CoordinationProposalRequest{ + WalletPublicKeyHash: walletPublicKeyHash, + } + + // First run: min amount (1000) is well below the deposit (2000000) - must accept. + _, shouldExecute, err := task.Run(request) + if err != nil { + t.Fatalf("unexpected error on first run: [%v]", err) + } + if !shouldExecute { + t.Fatalf("expected shouldExecute=true on first run, got false") + } + + // Second run without mutation: must accept again. + _, shouldExecute, err = task.Run(request) + if err != nil { + t.Fatalf("unexpected error on second run: [%v]", err) + } + if !shouldExecute { + t.Fatalf("expected shouldExecute=true on second run without mutation, got false") + } + }) + + t.Run("with parameter mutation rejects on subsequent run", func(t *testing.T) { + ralc := newReservationAcceptanceLocalChain() + btcChain := tbtcpg.NewLocalBitcoinChain() + + ralc.reservationParameters = &tbtc.ReservationParameters{ + ReservationVault: chain.Address( + "0xReservationVaultAddress1234567890abcdef12345678", + ), + ReservationMinAmount: 1000, + ReservationTxMaxFee: 5000, + MaxReservationsPerWallet: 5, + } + ralc.maxPerWalletAmount = 5000000 + ralc.maxSingleAmount = 5000000 + ralc.maxActive = 100 + + ralc.SetDepositMinAge(3600) + ralc.SetWallet( + walletPublicKeyHash, + &tbtc.WalletChainData{State: tbtc.StateLive}, + ) + + blockCounter := tbtcpg.NewMockBlockCounter() + blockCounter.SetCurrentBlock(currentBlock) + ralc.SetBlockCounter(blockCounter) + + setupEligibleDeposit( + t, + ralc, + btcChain, + walletPublicKeyHash, + currentBlock, + 2000000, + ) + + task := tbtcpg.NewReservationAcceptanceTask(ralc, btcChain) + request := &tbtc.CoordinationProposalRequest{ + WalletPublicKeyHash: walletPublicKeyHash, + } + + // First run: min amount (1000) is well below the deposit (2000000) - must accept. + _, shouldExecute, err := task.Run(request) + if err != nil { + t.Fatalf("unexpected error on first run: [%v]", err) + } + if !shouldExecute { + t.Fatalf("expected shouldExecute=true on first run, got false") + } + + // Mutate the chain fake's parameters in place - same task instance, + // same deposit, no new task created - then raise the min amount above + // the deposit's value. + ralc.reservationParameters.ReservationMinAmount = 3000000 + + // Second run: if ReservationParameters() were cached from the first + // run, this would still see ReservationMinAmount=1000 and wrongly + // accept again. + _, shouldExecute, err = task.Run(request) + if err != nil { + t.Fatalf("unexpected error on second run: [%v]", err) + } + if shouldExecute { + t.Fatalf( + "expected shouldExecute=false on second run after raising " + + "ReservationMinAmount above the deposit's value - a " + + "true result here means ReservationParameters() was " + + "cached from the first run instead of fetched live", + ) + } + }) +} + // TestReservationAcceptanceTask_BoundaryChecks exercises explicit -// at-limit/one-over-limit boundary crossings for the three eligibility -// caps in checkReservationAcceptanceEligibility -// (reservation_acceptance.go:424, :443, :475-478): -// MaxReservationsPerWallet, ReservationMinAmount, and -// ReservationMaxTotalAmount. TestReservationAcceptanceTask_BoundedLookback -// exercises these fields only as fixture data, never at their boundary -// value. +// at-limit/one-over-limit boundary crossings for the eligibility +// caps in checkReservationAcceptanceEligibility: +// - MaxReservationsPerWallet +// - ReservationMinAmount +// - ReservationMaxTotalAmount +// - ReservationMaxSingleAmount +// - MaxReservationsAmountPerWallet +// - ActiveReservationsCount +// as well as the net-of-fee minimum check in proposeReservationAcceptance. func TestReservationAcceptanceTask_BoundaryChecks(t *testing.T) { tests := map[string]struct { depositAmount uint64 @@ -923,6 +1277,11 @@ func TestReservationAcceptanceTask_BoundaryChecks(t *testing.T) { reservationMinAmount uint64 reservationMaxTotal uint64 reservationTotal uint64 + maxSingleAmount uint64 + maxPerWalletAmount uint64 + walletReservationsAmount uint64 + maxActive uint32 + activeCount uint32 expectAccept bool }{ "MaxReservationsPerWallet: below limit accepts": { @@ -943,16 +1302,23 @@ func TestReservationAcceptanceTask_BoundaryChecks(t *testing.T) { // requires depositAmount >= reservationMinAmount, but // proposeReservationAcceptance additionally requires the // *net-of-fee* anchor value (deposit - anchorFee) to also clear - // reservationMinAmount. The fixture's TX shape/1 sat/vByte rate - // (see SetEstimateSatPerVByteFee above) always estimates a 710 - // sat fee, so depositAmount is set to reservationMinAmount + 710 - // to land exactly on the net-of-fee boundary, not the gross one. + // reservationMinAmount. Even though the test fixture sets a 1 sat/vByte + // oracle rate, applyWalletTxFeeFloor (see fee.go) clamps the rate to + // minWalletTxSatPerVByteFee (5 sat/vByte), resulting in a 710 sat fee + // (5 * 142 vsize = testAnchorFeeSat). Deposit amounts are offset by + // testAnchorFeeSat to test the exact net-of-fee boundary. "ReservationMinAmount: exactly at minimum accepts": { - depositAmount: 100710, + depositAmount: 100000 + testAnchorFeeSat, maxReservationsPerWallet: 5, reservationMinAmount: 100000, expectAccept: true, }, + "ReservationMinAmount: gross clears but net-of-fee value does not": { + depositAmount: 100050, + maxReservationsPerWallet: 5, + reservationMinAmount: 100000, + expectAccept: false, + }, "ReservationMinAmount: one below minimum rejects": { depositAmount: 99999, maxReservationsPerWallet: 5, @@ -975,6 +1341,52 @@ func TestReservationAcceptanceTask_BoundaryChecks(t *testing.T) { reservationMaxTotal: 5000000, expectAccept: false, }, + "ReservationMaxSingleAmount: exactly at cap accepts": { + depositAmount: 5000000, + maxSingleAmount: 5000000, + maxReservationsPerWallet: 5, + reservationMinAmount: 1000, + expectAccept: true, + }, + "ReservationMaxSingleAmount: one over cap rejects": { + depositAmount: 5000001, + maxSingleAmount: 5000000, + maxReservationsPerWallet: 5, + reservationMinAmount: 1000, + expectAccept: false, + }, + "MaxReservationsAmountPerWallet: exactly at cap accepts": { + depositAmount: 2000000, + walletReservationsAmount: 3000000, + maxPerWalletAmount: 5000000, + maxReservationsPerWallet: 5, + reservationMinAmount: 1000, + expectAccept: true, + }, + "MaxReservationsAmountPerWallet: one over cap rejects": { + depositAmount: 2000000, + walletReservationsAmount: 3000001, + maxPerWalletAmount: 5000000, + maxReservationsPerWallet: 5, + reservationMinAmount: 1000, + expectAccept: false, + }, + "ActiveReservationsCount: below limit accepts": { + depositAmount: 2000000, + maxActive: 10, + activeCount: 9, + maxReservationsPerWallet: 5, + reservationMinAmount: 1000, + expectAccept: true, + }, + "ActiveReservationsCount: at limit rejects": { + depositAmount: 2000000, + maxActive: 10, + activeCount: 10, + maxReservationsPerWallet: 5, + reservationMinAmount: 1000, + expectAccept: false, + }, } for testName, test := range tests { @@ -998,8 +1410,19 @@ func TestReservationAcceptanceTask_BoundaryChecks(t *testing.T) { ReservationTotalAmount: test.reservationTotal, } ralc.maxPerWalletAmount = 50000000 + if test.maxPerWalletAmount != 0 { + ralc.maxPerWalletAmount = test.maxPerWalletAmount + } ralc.maxSingleAmount = 50000000 + if test.maxSingleAmount != 0 { + ralc.maxSingleAmount = test.maxSingleAmount + } ralc.maxActive = 100 + if test.maxActive != 0 { + ralc.maxActive = test.maxActive + } + ralc.activeCount = test.activeCount + ralc.walletReservationsAmount = test.walletReservationsAmount ralc.walletReservationsCount = test.walletReservationsCount ralc.SetDepositMinAge(3600) @@ -1013,58 +1436,14 @@ func TestReservationAcceptanceTask_BoundaryChecks(t *testing.T) { blockCounter.SetCurrentBlock(currentBlock) ralc.SetBlockCounter(blockCounter) - // Each subtest gets its own fresh ralc/btcChain instance - // (not a shared package-level registry), so a fixed hash is - // safe to reuse across cases. - fundingTxHash := hashFromString( - "4444444444444444444444444444444444444444444444444444444444444444", - ) - - dummyTx := &bitcoin.Transaction{ - Outputs: []*bitcoin.TransactionOutput{{ - Value: 0, - PublicKeyScript: append([]byte{0x00, 0x20}, make([]byte, 32)...), - }}, - } - btcChain.SetTransaction(fundingTxHash, dummyTx) - btcChain.SetTransactionConfirmations( - fundingTxHash, - tbtc.DepositSweepRequiredFundingTxConfirmations, - ) - ralc.SetDepositRequest( - fundingTxHash, - 0, - &tbtc.DepositChainRequest{ - Depositor: chain.Address("934b98637ca318a4d6e7ca6ffd1690b8e77df637"), - Amount: test.depositAmount, - RevealedAt: time.Now().Add(-2 * time.Hour), - SweptAt: time.Unix(0, 0), - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], - }, + setupEligibleDeposit( + t, + ralc, + btcChain, + walletPublicKeyHash, + currentBlock, + test.depositAmount, ) - depositKey := ralc.BuildDepositKey(fundingTxHash, 0) - ralc.reservedDeposits[depositKey.Text(16)] = true - - if err := ralc.AddPastDepositRevealedEvent( - &tbtc.DepositRevealedEventFilter{ - StartBlock: currentBlock - tbtcpg.ReservationAcceptanceLookBackBlocks, - EndBlock: ¤tBlock, - WalletPublicKeyHash: [][20]byte{walletPublicKeyHash}, - }, - &tbtc.DepositRevealedEvent{ - BlockNumber: 200000, - WalletPublicKeyHash: walletPublicKeyHash, - FundingTxHash: fundingTxHash, - FundingOutputIndex: 0, - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], - }, - ); err != nil { - t.Fatal(err) - } task := tbtcpg.NewReservationAcceptanceTask(ralc, btcChain) From 2c38e85d56bf4e2d0f1b082e6d901710431e7e34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Thu, 3 Sep 2026 14:38:26 +0000 Subject: [PATCH 7/9] fix(tbtcpg): prevent head-of-line-blocking DoS in reservation acceptance A candidate deposit that cleared the gross ReservationMinAmount but failed the net-of-fee check (or didn't cover the anchor fee at all) was returned as the selected candidate and then aborted proposal generation with a hard error. Nothing marked the deposit ineligible after the abort, so the same doomed deposit was re-selected on every subsequent Run(), permanently blocking the wallet's reservation queue until the deposit-reveal event aged out of the ~30-day look-back window. Move both fee viability checks (anchor fee coverage and net-of-fee minimum) into findReservationAcceptanceCandidate's selection loop, so a candidate that fails either check is skipped in favor of the next one instead of halting the pipeline. proposeReservationAcceptance now reuses the fee already computed during selection instead of recomputing and re-validating it. Also documents (does not change) the pre-existing fail-open handling of GetReservation errors: its interface contract signals "not found" as an error, so failing closed there would reject every brand-new candidate; a RequestNonce==1 assertion was added to TestReservationAcceptanceTask_GetReservationError to pin the intentional fallback. --- pkg/tbtcpg/reservation_acceptance.go | 88 ++++++++++++++++++++-------- 1 file changed, 64 insertions(+), 24 deletions(-) diff --git a/pkg/tbtcpg/reservation_acceptance.go b/pkg/tbtcpg/reservation_acceptance.go index 62818009f1..c0a3ebb03a 100644 --- a/pkg/tbtcpg/reservation_acceptance.go +++ b/pkg/tbtcpg/reservation_acceptance.go @@ -104,6 +104,7 @@ type reservationAcceptanceCandidate struct { ReservationParameters *tbtc.ReservationParameters TxMaxFee uint64 RequestNonce uint64 + AnchorFee int64 } // findReservationAcceptanceCandidate returns the first reserved deposit @@ -358,6 +359,20 @@ func (rat *ReservationAcceptanceTask) findReservationAcceptanceCandidate( } // Second & Third fix: check reservation state and derive RequestNonce. + // + // GetReservation's documented contract (chain.go) is "returns an + // error if the reservation was not found" -- unlike the sibling + // lookups above (GetDepositRequest's foundRequest bool, + // PastReservationAcceptanceRequestedEvents' empty-slice-on-none), + // this call has no way to distinguish "not found" (the expected, + // common case for a brand-new candidate) from a genuine query + // failure. Failing closed here like the siblings would reject + // every first-time candidate, since "not yet reserved" is itself + // signaled as an error. This is a deliberate fail-open deviation: + // any GetReservation error is treated as "not yet created" and + // requestNonce defaults to 1; see + // TestReservationAcceptanceTask_GetReservationError for the pinned + // RequestNonce == 1 fallback behavior. var requestNonce uint64 = 1 reservation, err := rat.chain.GetReservation(depositKey) if err != nil { @@ -390,6 +405,47 @@ func (rat *ReservationAcceptanceTask) findReservationAcceptanceCandidate( requestNonce = reservation.RequestNonce + 1 } + // Estimate the anchor fee and check net-of-fee viability here, as + // part of candidate selection, rather than after a single candidate + // has already been chosen. A candidate that fails this check is + // skipped in favor of the next one; nothing marks it retried, so + // leaving this check in proposeReservationAcceptance (which is + // called for exactly one already-selected candidate) would cause + // the same doomed deposit to be re-selected and abort on every + // subsequent Run() until it aged out of the look-back window. + anchorFee, err := estimateReservationAcceptanceFee( + rat.btcChain, + reservationParameters.ReservationTxMaxFee, + ) + if err != nil { + taskLogger.Errorf( + "failed to estimate reservation acceptance transaction fee for [%v]: [%v]", + depositKey, + err, + ) + continue + } + + anchorValue := int64(depositRequest.Amount) - anchorFee + if anchorValue <= 0 { + taskLogger.Infof( + "reserved deposit [%v] value [%d] does not cover anchor fee [%d]; skipping", + depositKey, + depositRequest.Amount, + anchorFee, + ) + continue + } + if uint64(anchorValue) < reservationParameters.ReservationMinAmount { + taskLogger.Infof( + "reserved deposit [%v] net-of-fee value [%d] below minimum [%d]; skipping", + depositKey, + anchorValue, + reservationParameters.ReservationMinAmount, + ) + continue + } + taskLogger.Infof( "selected reserved deposit [%v] for acceptance", depositKey, @@ -416,6 +472,7 @@ func (rat *ReservationAcceptanceTask) findReservationAcceptanceCandidate( ReservationParameters: reservationParameters, TxMaxFee: reservationParameters.ReservationTxMaxFee, RequestNonce: requestNonce, + AnchorFee: anchorFee, }, nil } @@ -505,30 +562,13 @@ func (rat *ReservationAcceptanceTask) proposeReservationAcceptance( taskLogger.Infof("preparing a reservation acceptance proposal") - anchorFee, err := estimateReservationAcceptanceFee( - rat.btcChain, - candidate.TxMaxFee, - ) - if err != nil { - return nil, false, fmt.Errorf( - "cannot estimate reservation acceptance transaction fee: [%v]", - err, - ) - } - - anchorValue := candidate.Deposit.Utxo.Value - anchorFee - if anchorValue <= 0 { - return nil, false, fmt.Errorf( - "deposit value [%d] does not cover anchor fee [%d]", - candidate.Deposit.Utxo.Value, - anchorFee, - ) - } - - if candidate.ReservationParameters != nil && - uint64(anchorValue) < candidate.ReservationParameters.ReservationMinAmount { - return nil, false, nil - } + // The anchor fee and its net-of-fee viability were already computed and + // validated during candidate selection in findReservationAcceptanceCandidate; + // re-checking here (after exactly one candidate has already been chosen) + // would abort this Run() outright on failure instead of trying the next + // candidate, causing the same doomed deposit to be re-selected on every + // subsequent Run() until it aged out of the look-back window. + anchorFee := candidate.AnchorFee taskLogger.Infof("anchor transaction fee: [%d]", anchorFee) From f20a3b356a500b8a441b54cd62c45492085622f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Thu, 3 Sep 2026 14:38:39 +0000 Subject: [PATCH 8/9] test(reservations): address multi-agent review findings Fixes 20 P1/P2/P3 findings from a multi-agent review of this PR's own test coverage: pkg/tbtcpg/reservation_acceptance_test.go: - Narrow the PastDepositRevealedEvents override to swallow only its claimed sentinel; add error-injection coverage. - Assert AnchorTransactionAssembly's returned proposal fields against the task-derived candidate, not just a hand-built fixture object; make the fixture's ValidateReservationAnchorProposal genuinely check the funding outpoint instead of always returning nil. - Delete the fixture's shadow ReservationParameters override/field; route through the base LocalChain's already-correct value-copy setter/getter instead. - Fix a test comment overclaiming exclusion is "strictly attributable" to one filter when multiple fixture gaps independently cause it. - Make the freshness-control test's RequestReservationAcceptance override actually record an event, so the dedup guard it's meant to exercise engages for real. - Pin RequestNonce==1 for the documented GetReservation fail-open path. - Make BoundaryChecks' three cap fields pointer-typed so a row can express production's real "0 = unlimited" semantic; add rows proving it. - Delete the dead reservedDeposits field/IsReservedDeposit override (zero production readers). - Add TestReservationAcceptanceTask_ValidateProposalError covering the previously-unexercised validateErr wrapping path. - Fix two misleading comments (wrong function attribution for the ReservationMinAmount gate; wrong description of the GetWallet error contract). - Add testReservationVaultAddress const, replacing 30+ inline literal duplicates. - Extract newBoundaryTestChain helper, replacing a ~13-line setup block duplicated across 16 call sites. pkg/tbtcpg/chain_test.go: - Fix binary.BigEndian.PutUint64 writing EndBlock bytes into the startBlock buffer instead of endBlock across 5 call sites, so EndBlock actually contributes to the event-filter cache key. pkg/chain/ethereum/tbtc_test.go: - Replace a stale tbtc.go line-range doc citation with a symbol reference. - Fold TestConvertReservationFromAbiType_DropsCumulativeReanchorFee into TestConvertReservationFromAbiType as a subtest, matching this file's one-test-per-converter convention. pkg/chain/ethereum/tbtc.go: - Add an in-tree TODO marking ValidateReservationAnchorProposal's deferred test coverage, since the docs describing that deferral don't exist in this checkout. --- pkg/chain/ethereum/tbtc.go | 4 + pkg/chain/ethereum/tbtc_test.go | 125 ++-- pkg/tbtcpg/chain_test.go | 10 +- pkg/tbtcpg/reservation_acceptance_test.go | 864 ++++++++++------------ 4 files changed, 480 insertions(+), 523 deletions(-) diff --git a/pkg/chain/ethereum/tbtc.go b/pkg/chain/ethereum/tbtc.go index 5f02098d1b..18a0407fe4 100644 --- a/pkg/chain/ethereum/tbtc.go +++ b/pkg/chain/ethereum/tbtc.go @@ -2525,6 +2525,10 @@ func (tc *TbtcChain) ReservationParameters() ( return convertReservationParametersFromAbiType(abiParameters), nil } +// TODO(test-coverage): ValidateReservationAnchorProposal has no direct unit +// test coverage. It requires go-ethereum simulated-backend infrastructure +// that does not exist anywhere in pkg/chain/ethereum today; blocked on that +// infra landing. See PR #4280 and its linked gap-analysis doc. // ValidateReservationAnchorProposal asks the WalletProposalValidator // whether the given anchor proposal is valid for the given wallet and // reserved deposit. The validator is a separate contract reached at its diff --git a/pkg/chain/ethereum/tbtc_test.go b/pkg/chain/ethereum/tbtc_test.go index 01ff179797..7540b8227c 100644 --- a/pkg/chain/ethereum/tbtc_test.go +++ b/pkg/chain/ethereum/tbtc_test.go @@ -556,8 +556,8 @@ func TestConvertReservationFromAbiType(t *testing.T) { RetryCredit: true, DissolutionEligibleAt: 1700200000, // CumulativeReanchorFee is intentionally dropped on the Go - // boundary (see the function doc comment); set it to a nonzero - // value to prove it never leaks into tbtc.Reservation. + // boundary (see the Field omissions note on + // convertReservationFromAbiType). CumulativeReanchorFee: 12345, } @@ -602,6 +602,66 @@ func TestConvertReservationFromAbiType(t *testing.T) { t.Fatal("expected error, got nil") } }) + + // t.Run below documents the intentional CumulativeReanchorFee drop + // performed by convertReservationFromAbiType: the field is written + // on-chain by every re-anchor hop but is not exposed on + // tbtc.Reservation (see the Field omissions note on + // convertReservationFromAbiType). It also pins that every other + // field maps correctly - each field below is a distinct value so a + // future accidental restoration of CumulativeReanchorFee, or a + // swapped adjacent field, does not go unnoticed. + t.Run("drops cumulative reanchor fee and maps every other field", func(t *testing.T) { + abiReservation := tbtcabi.ReservationReservationRequest{ + Owner: common.HexToAddress("0x111111111111111111111111111111111111111B"), + MintedAmount: 111, + AcceptedAt: 222, + WalletPubKeyHash: [20]byte{0x01, 0x02, 0x03}, + AnchorAmount: 333, + ExpiresAt: 444, + AnchorTxHash: [32]byte{0x04, 0x05, 0x06}, + AnchorTxOutputIndex: 555, + State: 1, // ReservationStateActive + RequestNonce: 666, + RetryCredit: true, + DissolutionEligibleAt: 777, + CumulativeReanchorFee: 888, // must not appear anywhere in the output + } + + expected := &tbtc.Reservation{ + Owner: chain.Address("0x111111111111111111111111111111111111111B"), + MintedAmount: 111, + AcceptedAt: 222, + WalletPublicKeyHash: [20]byte{ + 0x01, 0x02, 0x03, + }, + AnchorUtxo: &bitcoin.UnspentTransactionOutput{ + Outpoint: &bitcoin.TransactionOutpoint{ + TransactionHash: bitcoin.Hash{0x04, 0x05, 0x06}, + OutputIndex: 555, + }, + Value: 333, + }, + ExpiresAt: 444, + State: tbtc.ReservationStateActive, + RequestNonce: 666, + RetryCredit: true, + DissolutionEligibleAt: 777, + } + + actual, err := convertReservationFromAbiType(abiReservation) + if err != nil { + t.Fatal(err) + } + + if !reflect.DeepEqual(expected, actual) { + t.Errorf( + "unexpected reservation\nexpected: [%+v]\nactual: [%+v]", + expected, + actual, + ) + } + }) } func TestConvertReservationActionFromAbiType(t *testing.T) { @@ -806,64 +866,3 @@ func TestConvertReservationParametersFromAbiType(t *testing.T) { ) } } - -// TestConvertReservationFromAbiType_DropsCumulativeReanchorFee documents -// the intentional CumulativeReanchorFee drop performed by -// convertReservationFromAbiType: the field is written on-chain by every -// re-anchor hop but is not exposed on tbtc.Reservation because m1 has no -// fee-ceiling enforcement (own comment, tbtc.go:2637-2643). This test both -// pins that intentional omission and verifies every other field maps -// correctly - each field below is a distinct value so a future accidental -// restoration of CumulativeReanchorFee, or a swapped adjacent field, does -// not go unnoticed. -func TestConvertReservationFromAbiType_DropsCumulativeReanchorFee(t *testing.T) { - abiReservation := tbtcabi.ReservationReservationRequest{ - Owner: common.HexToAddress("0x111111111111111111111111111111111111111B"), - MintedAmount: 111, - AcceptedAt: 222, - WalletPubKeyHash: [20]byte{0x01, 0x02, 0x03}, - AnchorAmount: 333, - ExpiresAt: 444, - AnchorTxHash: [32]byte{0x04, 0x05, 0x06}, - AnchorTxOutputIndex: 555, - State: 1, // ReservationStateActive - RequestNonce: 666, - RetryCredit: true, - DissolutionEligibleAt: 777, - CumulativeReanchorFee: 888, // must not appear anywhere in the output - } - - expected := &tbtc.Reservation{ - Owner: chain.Address("0x111111111111111111111111111111111111111B"), - MintedAmount: 111, - AcceptedAt: 222, - WalletPublicKeyHash: [20]byte{ - 0x01, 0x02, 0x03, - }, - AnchorUtxo: &bitcoin.UnspentTransactionOutput{ - Outpoint: &bitcoin.TransactionOutpoint{ - TransactionHash: bitcoin.Hash{0x04, 0x05, 0x06}, - OutputIndex: 555, - }, - Value: 333, - }, - ExpiresAt: 444, - State: tbtc.ReservationStateActive, - RequestNonce: 666, - RetryCredit: true, - DissolutionEligibleAt: 777, - } - - actual, err := convertReservationFromAbiType(abiReservation) - if err != nil { - t.Fatal(err) - } - - if !reflect.DeepEqual(expected, actual) { - t.Errorf( - "unexpected reservation\nexpected: [%+v]\nactual: [%+v]", - expected, - actual, - ) - } -} diff --git a/pkg/tbtcpg/chain_test.go b/pkg/tbtcpg/chain_test.go index 69de27fa20..b592e36743 100644 --- a/pkg/tbtcpg/chain_test.go +++ b/pkg/tbtcpg/chain_test.go @@ -158,7 +158,7 @@ func buildPastDepositRevealedEventsKey( if filter.EndBlock != nil { endBlock := make([]byte, 8) - binary.BigEndian.PutUint64(startBlock, *filter.EndBlock) + binary.BigEndian.PutUint64(endBlock, *filter.EndBlock) buffer.Write(endBlock) } @@ -266,7 +266,7 @@ func buildPastNewWalletRegisteredEventsKey( if filter.EndBlock != nil { endBlock := make([]byte, 8) - binary.BigEndian.PutUint64(startBlock, *filter.EndBlock) + binary.BigEndian.PutUint64(endBlock, *filter.EndBlock) buffer.Write(endBlock) } @@ -335,7 +335,7 @@ func buildPastRedemptionRequestedEventsKey( if filter.EndBlock != nil { endBlock := make([]byte, 8) - binary.BigEndian.PutUint64(startBlock, *filter.EndBlock) + binary.BigEndian.PutUint64(endBlock, *filter.EndBlock) buffer.Write(endBlock) } @@ -370,7 +370,7 @@ func buildPastMovingFundsCommitmentSubmittedEventsKey( if filter.EndBlock != nil { endBlock := make([]byte, 8) - binary.BigEndian.PutUint64(startBlock, *filter.EndBlock) + binary.BigEndian.PutUint64(endBlock, *filter.EndBlock) buffer.Write(endBlock) } @@ -396,7 +396,7 @@ func buildPastMovingFundsCompletedEventsKey( if filter.EndBlock != nil { endBlock := make([]byte, 8) - binary.BigEndian.PutUint64(startBlock, *filter.EndBlock) + binary.BigEndian.PutUint64(endBlock, *filter.EndBlock) buffer.Write(endBlock) } diff --git a/pkg/tbtcpg/reservation_acceptance_test.go b/pkg/tbtcpg/reservation_acceptance_test.go index a14bd2635e..8e945f8923 100644 --- a/pkg/tbtcpg/reservation_acceptance_test.go +++ b/pkg/tbtcpg/reservation_acceptance_test.go @@ -26,6 +26,13 @@ import ( // applyWalletTxFeeFloor (see fee.go). const testAnchorFeeSat = uint64(710) +// testReservationVaultAddress is the reservation vault address used across +// this file's fixtures, so a deposit's Vault field targets the same vault +// configured in ReservationParameters.ReservationVault. +const testReservationVaultAddress = chain.Address( + "0xReservationVaultAddress1234567890abcdef12345678", +) + // reservationAcceptanceLocalChain is a test-only mock of tbtcpg.Chain that // embeds the production LocalChain and adds reservation-specific behavior. // It exists as a separate type so this test file does not need to edit the @@ -33,57 +40,51 @@ const testAnchorFeeSat = uint64(710) type reservationAcceptanceLocalChain struct { *tbtcpg.LocalChain - reservationParameters *tbtc.ReservationParameters - maxPerWalletAmount uint64 - maxSingleAmount uint64 - walletReservationsAmount uint64 - walletReservationsCount uint32 - activeCount uint32 - maxActive uint32 - pendingReserved uint64 - reservedDeposits map[string]bool - validateErr error - getWalletErr error - getReservationErr error - acceptanceEvents []*tbtc.ReservationAcceptanceRequestedEvent - acceptanceEventsErr error + maxPerWalletAmount uint64 + maxSingleAmount uint64 + walletReservationsAmount uint64 + walletReservationsCount uint32 + activeCount uint32 + maxActive uint32 + pendingReserved uint64 + validateErr error + getWalletErr error + getReservationErr error + acceptanceEvents []*tbtc.ReservationAcceptanceRequestedEvent + acceptanceEventsErr error + pastDepositRevealedEventsErr error } func newReservationAcceptanceLocalChain() *reservationAcceptanceLocalChain { lc := tbtcpg.NewLocalChain() return &reservationAcceptanceLocalChain{ - LocalChain: lc, - reservedDeposits: make(map[string]bool), + LocalChain: lc, } } // PastDepositRevealedEvents overrides the embedded LocalChain -// implementation to return an empty slice (rather than an error) when no -// events are registered for the filter. A real chain returns an empty -// event list when no deposits match; the in-memory mock's panic-stub -// "no events for given filter" error is a fixture bug that this override -// papers over without touching shared test infrastructure. +// implementation to narrow its "no events for given filter" sentinel +// error (the mock's signal for "nothing registered for this filter yet") +// into an empty slice, matching a real chain's behavior of returning an +// empty event list rather than an error when no deposits match. Any +// other error - including one injected via pastDepositRevealedEventsErr - +// is propagated unchanged. func (ralc *reservationAcceptanceLocalChain) PastDepositRevealedEvents( filter *tbtc.DepositRevealedEventFilter, ) ([]*tbtc.DepositRevealedEvent, error) { + if ralc.pastDepositRevealedEventsErr != nil { + return nil, ralc.pastDepositRevealedEventsErr + } events, err := ralc.LocalChain.PastDepositRevealedEvents(filter) if err != nil { - return []*tbtc.DepositRevealedEvent{}, nil + if err.Error() == "no events for given filter" { + return []*tbtc.DepositRevealedEvent{}, nil + } + return nil, err } return events, nil } -func (ralc *reservationAcceptanceLocalChain) ReservationParameters() ( - *tbtc.ReservationParameters, - error, -) { - if ralc.reservationParameters == nil { - return nil, nil - } - paramsCopy := *ralc.reservationParameters - return ¶msCopy, nil -} - func (ralc *reservationAcceptanceLocalChain) ReservationCaps() ( uint64, uint64, @@ -119,15 +120,6 @@ func (ralc *reservationAcceptanceLocalChain) PendingReservedDeposits() ( return ralc.pendingReserved, nil } -func (ralc *reservationAcceptanceLocalChain) IsReservedDeposit( - depositKey *big.Int, -) (bool, error) { - if depositKey == nil { - return false, nil - } - return ralc.reservedDeposits[depositKey.Text(16)], nil -} - func (ralc *reservationAcceptanceLocalChain) GetWallet( walletPublicKeyHash [20]byte, ) (*tbtc.WalletChainData, error) { @@ -146,6 +138,12 @@ func (ralc *reservationAcceptanceLocalChain) GetReservation( return ralc.LocalChain.GetReservation(reservationKey) } +// ValidateReservationAnchorProposal overrides the embedded LocalChain +// implementation. When validateErr is set it returns that error +// unconditionally (see TestReservationAcceptanceTask_ValidateProposalError). +// Otherwise it genuinely exercises the candidate-deposit mapping step by +// checking the proposal's funding outpoint against the candidate deposit's +// own funding outpoint, rather than unconditionally succeeding. func (ralc *reservationAcceptanceLocalChain) ValidateReservationAnchorProposal( walletPublicKeyHash [20]byte, proposal *tbtc.ReservationAnchorProposal, @@ -154,7 +152,34 @@ func (ralc *reservationAcceptanceLocalChain) ValidateReservationAnchorProposal( FundingTx *bitcoin.Transaction }, ) error { - return ralc.validateErr + if ralc.validateErr != nil { + return ralc.validateErr + } + if depositExtraInfo.Deposit == nil || + depositExtraInfo.Deposit.Utxo == nil || + depositExtraInfo.Deposit.Utxo.Outpoint == nil { + return fmt.Errorf( + "validate reservation anchor proposal: missing deposit UTXO outpoint", + ) + } + outpoint := depositExtraInfo.Deposit.Utxo.Outpoint + if outpoint.TransactionHash != proposal.DepositFundingTxHash { + return fmt.Errorf( + "validate reservation anchor proposal: funding tx hash mismatch: "+ + "proposal=[%x] candidate=[%x]", + proposal.DepositFundingTxHash, + outpoint.TransactionHash, + ) + } + if outpoint.OutputIndex != proposal.DepositFundingOutputIndex { + return fmt.Errorf( + "validate reservation anchor proposal: funding output index mismatch: "+ + "proposal=[%d] candidate=[%d]", + proposal.DepositFundingOutputIndex, + outpoint.OutputIndex, + ) + } + return nil } func (ralc *reservationAcceptanceLocalChain) PastReservationAcceptanceRequestedEvents( @@ -188,6 +213,26 @@ func (ralc *reservationAcceptanceLocalChain) AddPastReservationAcceptanceRequest ralc.acceptanceEvents = append(ralc.acceptanceEvents, event) } +// RequestReservationAcceptance overrides the embedded LocalChain no-op +// implementation to actually record a ReservationAcceptanceRequestedEvent, +// so that production's own dedup guard (which queries +// PastReservationAcceptanceRequestedEvents) genuinely engages on a +// subsequent Run() against the same reservation, instead of silently never +// observing the request this call represents. +func (ralc *reservationAcceptanceLocalChain) RequestReservationAcceptance( + reservationKey *big.Int, + walletPublicKeyHash [20]byte, +) error { + ralc.acceptanceEvents = append( + ralc.acceptanceEvents, + &tbtc.ReservationAcceptanceRequestedEvent{ + ReservationKey: reservationKey, + WalletPublicKeyHash: walletPublicKeyHash, + }, + ) + return nil +} + // scenarioReservationAcceptanceChain wires a scenario's on-chain state // into the test mock chain. func scenarioReservationAcceptanceChain( @@ -203,14 +248,14 @@ func scenarioReservationAcceptanceChain( reservationVault = chain.Address(scenario.ReservationVault) } - ralc.reservationParameters = &tbtc.ReservationParameters{ + ralc.SetReservationParameters(tbtc.ReservationParameters{ ReservationVault: reservationVault, ReservationMinAmount: scenario.ReservationParameters.ReservationMinAmount, ReservationTxMaxFee: scenario.ReservationParameters.ReservationTxMaxFee, ReservationMaxTotalAmount: scenario.ReservationParameters.ReservationMaxTotalAmount, ReservationTotalAmount: scenario.ReservationParameters.ReservationTotalAmount, MaxReservationsPerWallet: scenario.ReservationParameters.MaxReservationsPerWallet, - } + }) ralc.maxPerWalletAmount = scenario.Caps.MaxReservationsAmountPerWallet ralc.maxSingleAmount = scenario.Caps.ReservationMaxSingleAmount @@ -250,9 +295,8 @@ func scenarioReservationAcceptanceChain( } // registerReservedDeposits wires the scenario's reserved deposits into the -// mock chain as deposit requests and past DepositRevealedEvents. It also -// marks them as reserved via IsReservedDeposit. Bitcoin transaction -// registrations live on the btcChain mock. +// mock chain as deposit requests and past DepositRevealedEvents. Bitcoin +// transaction registrations live on the btcChain mock. func registerReservedDeposits( t *testing.T, scenario *test.ReservationAcceptanceTestScenario, @@ -337,13 +381,6 @@ func registerReservedDeposits( err, ) } - - depositKey := ralc.BuildDepositKey( - materialized.FundingTxHash, - materialized.FundingOutputIndex, - ) - // kept for parity with registerReservedDeposits; not read by ReservationAcceptanceTask - ralc.reservedDeposits[depositKey.Text(16)] = true } } @@ -377,11 +414,10 @@ func setupEligibleDeposit( tbtc.DepositSweepRequiredFundingTxConfirmations, ) - vaultAddress := chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - ) - if ralc.reservationParameters != nil && ralc.reservationParameters.ReservationVault != "" { - vaultAddress = ralc.reservationParameters.ReservationVault + vaultAddress := testReservationVaultAddress + if params, err := ralc.ReservationParameters(); err == nil && + params.ReservationVault != "" { + vaultAddress = params.ReservationVault } ralc.SetDepositRequest( @@ -427,6 +463,58 @@ func setupEligibleDeposit( return fundingTxHash } +// newBoundaryTestChain builds a reservationAcceptanceLocalChain with the +// reservation-parameters/caps/wallet/block-counter setup shared by most of +// this file's Run()-based tests: a live wallet at walletPublicKeyHash, a +// ReservationParameters of {vault: testReservationVaultAddress, minAmount: +// 1000, txMaxFee: 5000, maxPerWallet: 5}, per-wallet/single caps of +// 5000000, an active-reservations cap of 100, and a deposit minimum age of +// one hour. overrides, when non-nil, runs after these defaults so a call +// site can customize only what it varies (e.g. re-set ReservationParameters +// with different values, raise a cap, or inject an error field). +func newBoundaryTestChain( + t *testing.T, + walletPublicKeyHash [20]byte, + currentBlock uint64, + overrides func(ralc *reservationAcceptanceLocalChain), +) *reservationAcceptanceLocalChain { + t.Helper() + + ralc := newReservationAcceptanceLocalChain() + + ralc.SetReservationParameters(tbtc.ReservationParameters{ + ReservationVault: testReservationVaultAddress, + ReservationMinAmount: 1000, + ReservationTxMaxFee: 5000, + MaxReservationsPerWallet: 5, + }) + ralc.maxPerWalletAmount = 5000000 + ralc.maxSingleAmount = 5000000 + ralc.maxActive = 100 + + ralc.SetDepositMinAge(3600) + ralc.SetWallet( + walletPublicKeyHash, + &tbtc.WalletChainData{State: tbtc.StateLive}, + ) + + blockCounter := tbtcpg.NewMockBlockCounter() + blockCounter.SetCurrentBlock(currentBlock) + ralc.SetBlockCounter(blockCounter) + + if overrides != nil { + overrides(ralc) + } + + return ralc +} + +// uint64Ptr and uint32Ptr let a TestReservationAcceptanceTask_BoundaryChecks +// table row distinguish an explicit cap value of 0 (production's +// "unlimited" semantic for these caps) from the field's unset zero value. +func uint64Ptr(v uint64) *uint64 { return &v } +func uint32Ptr(v uint32) *uint32 { return &v } + // expectedAnchorsEqual compares two proposal objects field-by-field. // deep.Equal cannot be used for this: by default it does not descend into // unexported fields, and *big.Int's representation is entirely unexported, @@ -575,7 +663,6 @@ func TestReservationAcceptanceTask_Run(t *testing.T) { // Bitcoin transaction paying the correct wallet P2WPKH output script with // value equal to deposit amount minus the estimated anchor fee. func TestReservationAcceptanceTask_AnchorTransactionAssembly(t *testing.T) { - ralc := newReservationAcceptanceLocalChain() btcChain := tbtcpg.NewLocalBitcoinChain() btcChain.SetEstimateSatPerVByteFee(1, 1) @@ -588,27 +675,10 @@ func TestReservationAcceptanceTask_AnchorTransactionAssembly(t *testing.T) { depositAmount := uint64(2000000) currentBlock := uint64(300000) - ralc.reservationParameters = &tbtc.ReservationParameters{ - ReservationVault: chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - ), - ReservationMinAmount: 1000, - ReservationTxMaxFee: 5000, - MaxReservationsPerWallet: 5, - } - ralc.maxPerWalletAmount = 50000000 - ralc.maxSingleAmount = 50000000 - ralc.maxActive = 100 - - ralc.SetDepositMinAge(3600) - ralc.SetWallet( - walletPublicKeyHash, - &tbtc.WalletChainData{State: tbtc.StateLive}, - ) - - blockCounter := tbtcpg.NewMockBlockCounter() - blockCounter.SetCurrentBlock(currentBlock) - ralc.SetBlockCounter(blockCounter) + ralc := newBoundaryTestChain(t, walletPublicKeyHash, currentBlock, func(ralc *reservationAcceptanceLocalChain) { + ralc.maxPerWalletAmount = 50000000 + ralc.maxSingleAmount = 50000000 + }) deposit := &tbtc.Deposit{ Depositor: chain.Address("934b98637ca318a4d6e7ca6ffd1690b8e77df637"), @@ -616,9 +686,7 @@ func TestReservationAcceptanceTask_AnchorTransactionAssembly(t *testing.T) { WalletPublicKeyHash: walletPublicKeyHash, RefundPublicKeyHash: [20]byte{0x02}, RefundLocktime: [4]byte{0x03, 0x04, 0x05, 0x06}, - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], + Vault: &[]chain.Address{testReservationVaultAddress}[0], } depositScript, err := deposit.Script() @@ -717,6 +785,24 @@ func TestReservationAcceptanceTask_AnchorTransactionAssembly(t *testing.T) { t.Fatalf("expected *ReservationAnchorProposal, got %T", proposal) } + // Assert on the candidate-derived proposal's own fields, exercising the + // candidate-deposit mapping step (also checked by the fixture's + // ValidateReservationAnchorProposal override), rather than only + // reassembling from this test's own hand-built deposit object below. + if anchorProposal.DepositFundingTxHash != fundingTxHash { + t.Errorf( + "unexpected DepositFundingTxHash\nexpected: %x\nactual: %x", + fundingTxHash, + anchorProposal.DepositFundingTxHash, + ) + } + if anchorProposal.DepositFundingOutputIndex != 0 { + t.Errorf( + "unexpected DepositFundingOutputIndex\nexpected: 0\nactual: %d", + anchorProposal.DepositFundingOutputIndex, + ) + } + // Re-assemble and sign to verify transaction builder output properties. builder, err := tbtc.AssembleReservationAnchorTransaction( btcChain, @@ -783,32 +869,19 @@ func TestReservationAcceptanceTask_AnchorTransactionAssembly(t *testing.T) { // TestReservationAcceptanceTask_NoCandidates verifies that the task is a // no-op when the chain has no reserved deposits. func TestReservationAcceptanceTask_NoCandidates(t *testing.T) { - ralc := newReservationAcceptanceLocalChain() btcChain := tbtcpg.NewLocalBitcoinChain() walletPublicKeyHash := hexToByte20( "8db50eb52063ea9d98b3eac91489a90f738986f6", ) - - ralc.reservationParameters = &tbtc.ReservationParameters{ - ReservationVault: chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - ), - } - ralc.maxPerWalletAmount = 1000000 - ralc.maxSingleAmount = 5000000 - ralc.maxActive = 100 - - ralc.SetDepositMinAge(3600) - ralc.SetWallet( - walletPublicKeyHash, - &tbtc.WalletChainData{State: tbtc.StateLive}, - ) - currentBlock := uint64(300000) - blockCounter := tbtcpg.NewMockBlockCounter() - blockCounter.SetCurrentBlock(currentBlock) - ralc.SetBlockCounter(blockCounter) + + ralc := newBoundaryTestChain(t, walletPublicKeyHash, currentBlock, func(ralc *reservationAcceptanceLocalChain) { + ralc.SetReservationParameters(tbtc.ReservationParameters{ + ReservationVault: testReservationVaultAddress, + }) + ralc.maxPerWalletAmount = 1000000 + }) task := tbtcpg.NewReservationAcceptanceTask(ralc, btcChain) @@ -833,34 +906,13 @@ func TestReservationAcceptanceTask_NoCandidates(t *testing.T) { func TestReservationAcceptanceTask_BoundedLookback(t *testing.T) { currentBlock := uint64(400000) - ralc := newReservationAcceptanceLocalChain() btcChain := tbtcpg.NewLocalBitcoinChain() walletPublicKeyHash := hexToByte20( "8db50eb52063ea9d98b3eac91489a90f738986f6", ) - ralc.reservationParameters = &tbtc.ReservationParameters{ - ReservationVault: chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - ), - ReservationMinAmount: 1000, - ReservationTxMaxFee: 5000, - MaxReservationsPerWallet: 5, - } - ralc.maxPerWalletAmount = 5000000 - ralc.maxSingleAmount = 5000000 - ralc.maxActive = 100 - - ralc.SetDepositMinAge(3600) - ralc.SetWallet( - walletPublicKeyHash, - &tbtc.WalletChainData{State: tbtc.StateLive}, - ) - - blockCounter := tbtcpg.NewMockBlockCounter() - blockCounter.SetCurrentBlock(currentBlock) - ralc.SetBlockCounter(blockCounter) + ralc := newBoundaryTestChain(t, walletPublicKeyHash, currentBlock, nil) // Register an event below the look-back start block (block 1). oldFundingTxHash := hashFromString( @@ -887,10 +939,11 @@ func TestReservationAcceptanceTask_BoundedLookback(t *testing.T) { WalletPublicKeyHash: walletPublicKeyHash, } - // First run: only the old deposit exists below the look-back window. - // Because the candidate satisfies all other conditions (valid wallet, - // clear caps), the shouldExecute=false result is strictly attributable - // to exclusion by the look-back start block filter. + // First run: only the old deposit exists, revealed at block 1 - before + // the look-back start block. No candidate is found on this run; the + // second run below is what actually proves the look-back start block + // is honored, by registering an eligible deposit exactly at that block + // and confirming it is then found and accepted. proposal, shouldExecute, err := task.Run(request) if err != nil { t.Fatalf("unexpected error on old deposit run: [%v]", err) @@ -943,32 +996,18 @@ func TestReservationAcceptanceTask_BoundedLookback(t *testing.T) { // TestReservationAcceptanceTask_DepositNotReserved confirms that a deposit // that does not target the reservation vault is filtered out. func TestReservationAcceptanceTask_DepositNotReserved(t *testing.T) { - ralc := newReservationAcceptanceLocalChain() btcChain := tbtcpg.NewLocalBitcoinChain() walletPublicKeyHash := hexToByte20( "8db50eb52063ea9d98b3eac91489a90f738986f6", ) - - ralc.reservationParameters = &tbtc.ReservationParameters{ - ReservationVault: chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - ), - } - ralc.maxPerWalletAmount = 5000000 - ralc.maxSingleAmount = 5000000 - ralc.maxActive = 100 - - ralc.SetDepositMinAge(3600) - ralc.SetWallet( - walletPublicKeyHash, - &tbtc.WalletChainData{State: tbtc.StateLive}, - ) - currentBlock := uint64(300000) - blockCounter := tbtcpg.NewMockBlockCounter() - blockCounter.SetCurrentBlock(currentBlock) - ralc.SetBlockCounter(blockCounter) + + ralc := newBoundaryTestChain(t, walletPublicKeyHash, currentBlock, func(ralc *reservationAcceptanceLocalChain) { + ralc.SetReservationParameters(tbtc.ReservationParameters{ + ReservationVault: testReservationVaultAddress, + }) + }) fundingTxHash := hashFromString( "3333333333333333333333333333333333333333333333333333333333333333", @@ -1037,34 +1076,19 @@ func TestReservationAcceptanceTask_DepositNotReserved(t *testing.T) { // reserved deposit candidate is discovered and matches the reservation // vault, but the candidate wallet's chain data fails to load. func TestReservationAcceptanceTask_GetWalletError(t *testing.T) { - ralc := newReservationAcceptanceLocalChain() btcChain := tbtcpg.NewLocalBitcoinChain() walletPublicKeyHash := hexToByte20( "8db50eb52063ea9d98b3eac91489a90f738986f6", ) - - ralc.reservationParameters = &tbtc.ReservationParameters{ - ReservationVault: chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - ), - ReservationMinAmount: 1000, - ReservationTxMaxFee: 5000, - MaxReservationsPerWallet: 5, - } - ralc.maxPerWalletAmount = 5000000 - ralc.maxSingleAmount = 5000000 - ralc.maxActive = 100 - - ralc.SetDepositMinAge(3600) - // No SetWallet call: GetWallet fails for the candidate wallet, and - // getWalletErr forces the exact error to assert against. - ralc.getWalletErr = fmt.Errorf("boom") - currentBlock := uint64(300000) - blockCounter := tbtcpg.NewMockBlockCounter() - blockCounter.SetCurrentBlock(currentBlock) - ralc.SetBlockCounter(blockCounter) + + // getWalletErr forces GetWallet to fail for the candidate wallet. + // Production logs and swallows the GetWallet error, so Run must + // return (nil, false, nil). + ralc := newBoundaryTestChain(t, walletPublicKeyHash, currentBlock, func(ralc *reservationAcceptanceLocalChain) { + ralc.getWalletErr = fmt.Errorf("boom") + }) setupEligibleDeposit( t, @@ -1095,35 +1119,14 @@ func TestReservationAcceptanceTask_GetWalletError(t *testing.T) { // GetReservation fails, the task logs the error and falls through to // RequestReservationAcceptance, continuing with proposal emission. func TestReservationAcceptanceTask_GetReservationError(t *testing.T) { - ralc := newReservationAcceptanceLocalChain() btcChain := tbtcpg.NewLocalBitcoinChain() walletPublicKeyHash := hexToByte20( "8db50eb52063ea9d98b3eac91489a90f738986f6", ) - - ralc.reservationParameters = &tbtc.ReservationParameters{ - ReservationVault: chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - ), - ReservationMinAmount: 1000, - ReservationTxMaxFee: 5000, - MaxReservationsPerWallet: 5, - } - ralc.maxPerWalletAmount = 5000000 - ralc.maxSingleAmount = 5000000 - ralc.maxActive = 100 - - ralc.SetDepositMinAge(3600) - ralc.SetWallet( - walletPublicKeyHash, - &tbtc.WalletChainData{State: tbtc.StateLive}, - ) - currentBlock := uint64(300000) - blockCounter := tbtcpg.NewMockBlockCounter() - blockCounter.SetCurrentBlock(currentBlock) - ralc.SetBlockCounter(blockCounter) + + ralc := newBoundaryTestChain(t, walletPublicKeyHash, currentBlock, nil) fundingTxHash := setupEligibleDeposit( t, @@ -1163,6 +1166,15 @@ func TestReservationAcceptanceTask_GetReservationError(t *testing.T) { actualProposal.DepositFundingTxHash.Hex(bitcoin.ReversedByteOrder), ) } + // GetReservation's error is intentionally fail-open (see production + // comment above the call site): a brand-new candidate's first + // acceptance request nonce defaults to 1. + if actualProposal.RequestNonce != 1 { + t.Errorf( + "unexpected RequestNonce\nexpected: 1\nactual: %d", + actualProposal.RequestNonce, + ) + } } // TestReservationAcceptanceTask_Stateless_Maturity verifies the stateless @@ -1171,35 +1183,14 @@ func TestReservationAcceptanceTask_GetReservationError(t *testing.T) { // the candidate matures, the second run on the same task instance proposes it // without any cache-state interference. func TestReservationAcceptanceTask_Stateless_Maturity(t *testing.T) { - ralc := newReservationAcceptanceLocalChain() btcChain := tbtcpg.NewLocalBitcoinChain() walletPublicKeyHash := hexToByte20( "8db50eb52063ea9d98b3eac91489a90f738986f6", ) - - ralc.reservationParameters = &tbtc.ReservationParameters{ - ReservationVault: chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - ), - ReservationMinAmount: 1000, - ReservationTxMaxFee: 5000, - MaxReservationsPerWallet: 5, - } - ralc.maxPerWalletAmount = 5000000 - ralc.maxSingleAmount = 5000000 - ralc.maxActive = 100 - - ralc.SetDepositMinAge(3600) - ralc.SetWallet( - walletPublicKeyHash, - &tbtc.WalletChainData{State: tbtc.StateLive}, - ) - currentBlock := uint64(300000) - blockCounter := tbtcpg.NewMockBlockCounter() - blockCounter.SetCurrentBlock(currentBlock) - ralc.SetBlockCounter(blockCounter) + + ralc := newBoundaryTestChain(t, walletPublicKeyHash, currentBlock, nil) fundingTxHash := hashFromString( "5555555555555555555555555555555555555555555555555555555555555555", @@ -1226,9 +1217,7 @@ func TestReservationAcceptanceTask_Stateless_Maturity(t *testing.T) { Amount: 2000000, RevealedAt: time.Now().Add(-10 * time.Minute), SweptAt: time.Unix(0, 0), - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], + Vault: &[]chain.Address{testReservationVaultAddress}[0], }, ) @@ -1248,9 +1237,7 @@ func TestReservationAcceptanceTask_Stateless_Maturity(t *testing.T) { WalletPublicKeyHash: walletPublicKeyHash, FundingTxHash: fundingTxHash, FundingOutputIndex: 0, - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], + Vault: &[]chain.Address{testReservationVaultAddress}[0], }, ); err != nil { t.Fatal(err) @@ -1279,9 +1266,7 @@ func TestReservationAcceptanceTask_Stateless_Maturity(t *testing.T) { Amount: 2000000, RevealedAt: time.Now().Add(-2 * time.Hour), SweptAt: time.Unix(0, 0), - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], + Vault: &[]chain.Address{testReservationVaultAddress}[0], }, ) @@ -1308,41 +1293,21 @@ func TestReservationAcceptanceTask_Stateless_Maturity(t *testing.T) { } // TestReservationAcceptanceTask_ReservationParametersFetchedLive verifies -// that ReservationParameters() is read fresh on every Run() call: a -// governance-driven parameter change must take effect on the very next -// call to the same task instance, with no leftover value from a prior run -// observable anywhere in the eligibility decision. +// that each Run() call reflects the chain's current live state rather than +// anything cached from a prior run on the same task instance: a +// governance-driven ReservationParameters change takes effect on the very +// next call, and an acceptance request recorded as a side effect of one +// Run() is visible to production's dedup guard on the next. func TestReservationAcceptanceTask_ReservationParametersFetchedLive(t *testing.T) { walletPublicKeyHash := hexToByte20( "8db50eb52063ea9d98b3eac91489a90f738986f6", ) currentBlock := uint64(300000) - t.Run("without parameter mutation accepts on subsequent run", func(t *testing.T) { - ralc := newReservationAcceptanceLocalChain() + t.Run("records acceptance request, skipping duplicate on subsequent run", func(t *testing.T) { btcChain := tbtcpg.NewLocalBitcoinChain() - ralc.reservationParameters = &tbtc.ReservationParameters{ - ReservationVault: chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - ), - ReservationMinAmount: 1000, - ReservationTxMaxFee: 5000, - MaxReservationsPerWallet: 5, - } - ralc.maxPerWalletAmount = 5000000 - ralc.maxSingleAmount = 5000000 - ralc.maxActive = 100 - - ralc.SetDepositMinAge(3600) - ralc.SetWallet( - walletPublicKeyHash, - &tbtc.WalletChainData{State: tbtc.StateLive}, - ) - - blockCounter := tbtcpg.NewMockBlockCounter() - blockCounter.SetCurrentBlock(currentBlock) - ralc.SetBlockCounter(blockCounter) + ralc := newBoundaryTestChain(t, walletPublicKeyHash, currentBlock, nil) setupEligibleDeposit( t, @@ -1367,41 +1332,25 @@ func TestReservationAcceptanceTask_ReservationParametersFetchedLive(t *testing.T t.Fatalf("expected shouldExecute=true on first run, got false") } - // Second run without mutation: must accept again. + // Second run on the same, now-requested deposit: RequestReservationAcceptance + // recorded a ReservationAcceptanceRequestedEvent as a side effect of the + // first run, so production's dedup guard (which queries + // PastReservationAcceptanceRequestedEvents) must now find it and skip the + // candidate. Without the fixture actually recording that event, this run + // would (incorrectly) accept again and the dedup guard would go untested. _, shouldExecute, err = task.Run(request) if err != nil { t.Fatalf("unexpected error on second run: [%v]", err) } - if !shouldExecute { - t.Fatalf("expected shouldExecute=true on second run without mutation, got false") + if shouldExecute { + t.Fatalf("expected shouldExecute=false on second run due to existing acceptance request, got true") } }) t.Run("with parameter mutation rejects on subsequent run", func(t *testing.T) { - ralc := newReservationAcceptanceLocalChain() btcChain := tbtcpg.NewLocalBitcoinChain() - ralc.reservationParameters = &tbtc.ReservationParameters{ - ReservationVault: chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - ), - ReservationMinAmount: 1000, - ReservationTxMaxFee: 5000, - MaxReservationsPerWallet: 5, - } - ralc.maxPerWalletAmount = 5000000 - ralc.maxSingleAmount = 5000000 - ralc.maxActive = 100 - - ralc.SetDepositMinAge(3600) - ralc.SetWallet( - walletPublicKeyHash, - &tbtc.WalletChainData{State: tbtc.StateLive}, - ) - - blockCounter := tbtcpg.NewMockBlockCounter() - blockCounter.SetCurrentBlock(currentBlock) - ralc.SetBlockCounter(blockCounter) + ralc := newBoundaryTestChain(t, walletPublicKeyHash, currentBlock, nil) setupEligibleDeposit( t, @@ -1429,7 +1378,12 @@ func TestReservationAcceptanceTask_ReservationParametersFetchedLive(t *testing.T // Mutate the chain fake's parameters in place - same task instance, // same deposit, no new task created - then raise the min amount above // the deposit's value. - ralc.reservationParameters.ReservationMinAmount = 3000000 + ralc.SetReservationParameters(tbtc.ReservationParameters{ + ReservationVault: testReservationVaultAddress, + ReservationMinAmount: 3000000, + ReservationTxMaxFee: 5000, + MaxReservationsPerWallet: 5, + }) // Second run: if any part of the eligibility path retained the // first run's ReservationMinAmount=1000 instead of reading the @@ -1448,15 +1402,18 @@ func TestReservationAcceptanceTask_ReservationParametersFetchedLive(t *testing.T } // TestReservationAcceptanceTask_BoundaryChecks exercises explicit -// at-limit/one-over-limit boundary crossings for the eligibility -// caps in checkReservationAcceptanceEligibility: +// at-limit/one-over-limit boundary crossings for the eligibility caps in +// checkReservationAcceptanceEligibility: // - MaxReservationsPerWallet -// - ReservationMinAmount // - ReservationMaxTotalAmount // - ReservationMaxSingleAmount // - MaxReservationsAmountPerWallet // - ActiveReservationsCount -// as well as the net-of-fee minimum check in proposeReservationAcceptance. +// ReservationMinAmount is not one of checkReservationAcceptanceEligibility's +// gates: the gross gate lives in findReservationAcceptanceCandidate, which +// requires depositAmount >= ReservationMinAmount; the same function +// additionally requires the net-of-fee value (deposit minus the estimated +// anchor fee) to clear it too. func TestReservationAcceptanceTask_BoundaryChecks(t *testing.T) { tests := map[string]struct { depositAmount uint64 @@ -1465,10 +1422,14 @@ func TestReservationAcceptanceTask_BoundaryChecks(t *testing.T) { reservationMinAmount uint64 reservationMaxTotal uint64 reservationTotal uint64 - maxSingleAmount uint64 - maxPerWalletAmount uint64 + // maxSingleAmount, maxPerWalletAmount, and maxActive are pointers + // so a test row can explicitly request the cap-disabled value of 0 + // (production's "0 = unlimited" semantic for these caps); nil means + // "use this test's default cap" instead. + maxSingleAmount *uint64 + maxPerWalletAmount *uint64 walletReservationsAmount uint64 - maxActive uint32 + maxActive *uint32 activeCount uint32 expectAccept bool }{ @@ -1531,22 +1492,33 @@ func TestReservationAcceptanceTask_BoundaryChecks(t *testing.T) { }, "ReservationMaxSingleAmount: exactly at cap accepts": { depositAmount: 5000000, - maxSingleAmount: 5000000, + maxSingleAmount: uint64Ptr(5000000), maxReservationsPerWallet: 5, reservationMinAmount: 1000, expectAccept: true, }, "ReservationMaxSingleAmount: one over cap rejects": { depositAmount: 5000001, - maxSingleAmount: 5000000, + maxSingleAmount: uint64Ptr(5000000), maxReservationsPerWallet: 5, reservationMinAmount: 1000, expectAccept: false, }, + // A cap of 0 means "unlimited" in checkReservationAcceptanceEligibility + // (reservationMaxSingleAmount > 0 gates the check); maxPerWalletAmount + // is raised explicitly so it does not itself gate this deposit. + "ReservationMaxSingleAmount: cap of 0 means unlimited": { + depositAmount: 60000000, + maxSingleAmount: uint64Ptr(0), + maxPerWalletAmount: uint64Ptr(100000000), + maxReservationsPerWallet: 5, + reservationMinAmount: 1000, + expectAccept: true, + }, "MaxReservationsAmountPerWallet: exactly at cap accepts": { depositAmount: 2000000, walletReservationsAmount: 3000000, - maxPerWalletAmount: 5000000, + maxPerWalletAmount: uint64Ptr(5000000), maxReservationsPerWallet: 5, reservationMinAmount: 1000, expectAccept: true, @@ -1554,14 +1526,26 @@ func TestReservationAcceptanceTask_BoundaryChecks(t *testing.T) { "MaxReservationsAmountPerWallet: one over cap rejects": { depositAmount: 2000000, walletReservationsAmount: 3000001, - maxPerWalletAmount: 5000000, + maxPerWalletAmount: uint64Ptr(5000000), maxReservationsPerWallet: 5, reservationMinAmount: 1000, expectAccept: false, }, + // A cap of 0 means "unlimited" (maxReservationsAmountPerWallet > 0 + // gates the check); maxSingleAmount is raised explicitly so it does + // not itself gate this deposit. + "MaxReservationsAmountPerWallet: cap of 0 means unlimited": { + depositAmount: 2000000, + walletReservationsAmount: 60000000, + maxPerWalletAmount: uint64Ptr(0), + maxSingleAmount: uint64Ptr(100000000), + maxReservationsPerWallet: 5, + reservationMinAmount: 1000, + expectAccept: true, + }, "ActiveReservationsCount: below limit accepts": { depositAmount: 2000000, - maxActive: 10, + maxActive: uint32Ptr(10), activeCount: 9, maxReservationsPerWallet: 5, reservationMinAmount: 1000, @@ -1569,12 +1553,22 @@ func TestReservationAcceptanceTask_BoundaryChecks(t *testing.T) { }, "ActiveReservationsCount: at limit rejects": { depositAmount: 2000000, - maxActive: 10, + maxActive: uint32Ptr(10), activeCount: 10, maxReservationsPerWallet: 5, reservationMinAmount: 1000, expectAccept: false, }, + // A cap of 0 means "unlimited" (maxActiveReservations > 0 gates the + // check). + "ActiveReservationsCount: cap of 0 means unlimited": { + depositAmount: 2000000, + maxActive: uint32Ptr(0), + activeCount: 1000, + maxReservationsPerWallet: 5, + reservationMinAmount: 1000, + expectAccept: true, + }, } for testName, test := range tests { @@ -1587,27 +1581,25 @@ func TestReservationAcceptanceTask_BoundaryChecks(t *testing.T) { "8db50eb52063ea9d98b3eac91489a90f738986f6", ) - ralc.reservationParameters = &tbtc.ReservationParameters{ - ReservationVault: chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - ), + ralc.SetReservationParameters(tbtc.ReservationParameters{ + ReservationVault: testReservationVaultAddress, ReservationMinAmount: test.reservationMinAmount, ReservationTxMaxFee: 5000, MaxReservationsPerWallet: test.maxReservationsPerWallet, ReservationMaxTotalAmount: test.reservationMaxTotal, ReservationTotalAmount: test.reservationTotal, - } + }) ralc.maxPerWalletAmount = 50000000 - if test.maxPerWalletAmount != 0 { - ralc.maxPerWalletAmount = test.maxPerWalletAmount + if test.maxPerWalletAmount != nil { + ralc.maxPerWalletAmount = *test.maxPerWalletAmount } ralc.maxSingleAmount = 50000000 - if test.maxSingleAmount != 0 { - ralc.maxSingleAmount = test.maxSingleAmount + if test.maxSingleAmount != nil { + ralc.maxSingleAmount = *test.maxSingleAmount } ralc.maxActive = 100 - if test.maxActive != 0 { - ralc.maxActive = test.maxActive + if test.maxActive != nil { + ralc.maxActive = *test.maxActive } ralc.activeCount = test.activeCount ralc.walletReservationsAmount = test.walletReservationsAmount @@ -1658,35 +1650,14 @@ func TestReservationAcceptanceTask_BoundaryChecks(t *testing.T) { // reservation has an existing acceptance requested event, subsequent Run calls // on the same task instance do not produce a duplicate acceptance proposal. func TestReservationAcceptanceTask_Stateless_NoReRequest(t *testing.T) { - ralc := newReservationAcceptanceLocalChain() btcChain := tbtcpg.NewLocalBitcoinChain() walletPublicKeyHash := hexToByte20( "8db50eb52063ea9d98b3eac91489a90f738986f6", ) - - ralc.reservationParameters = &tbtc.ReservationParameters{ - ReservationVault: chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - ), - ReservationMinAmount: 1000, - ReservationTxMaxFee: 5000, - MaxReservationsPerWallet: 5, - } - ralc.maxPerWalletAmount = 5000000 - ralc.maxSingleAmount = 5000000 - ralc.maxActive = 100 - - ralc.SetDepositMinAge(3600) - ralc.SetWallet( - walletPublicKeyHash, - &tbtc.WalletChainData{State: tbtc.StateLive}, - ) - currentBlock := uint64(300000) - blockCounter := tbtcpg.NewMockBlockCounter() - blockCounter.SetCurrentBlock(currentBlock) - ralc.SetBlockCounter(blockCounter) + + ralc := newBoundaryTestChain(t, walletPublicKeyHash, currentBlock, nil) fundingTxHash := hashFromString( "6666666666666666666666666666666666666666666666666666666666666666", @@ -1712,9 +1683,7 @@ func TestReservationAcceptanceTask_Stateless_NoReRequest(t *testing.T) { Amount: 2000000, RevealedAt: time.Now().Add(-2 * time.Hour), SweptAt: time.Unix(0, 0), - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], + Vault: &[]chain.Address{testReservationVaultAddress}[0], }, ) @@ -1734,9 +1703,7 @@ func TestReservationAcceptanceTask_Stateless_NoReRequest(t *testing.T) { WalletPublicKeyHash: walletPublicKeyHash, FundingTxHash: fundingTxHash, FundingOutputIndex: 0, - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], + Vault: &[]chain.Address{testReservationVaultAddress}[0], }, ); err != nil { t.Fatal(err) @@ -1778,35 +1745,14 @@ func TestReservationAcceptanceTask_Stateless_NoReRequest(t *testing.T) { // TestReservationAcceptanceTask_Stateless_PastEventsError verifies that an // RPC failure querying past acceptance requested events fails closed (skips candidate). func TestReservationAcceptanceTask_Stateless_PastEventsError(t *testing.T) { - ralc := newReservationAcceptanceLocalChain() btcChain := tbtcpg.NewLocalBitcoinChain() walletPublicKeyHash := hexToByte20( "8db50eb52063ea9d98b3eac91489a90f738986f6", ) - - ralc.reservationParameters = &tbtc.ReservationParameters{ - ReservationVault: chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - ), - ReservationMinAmount: 1000, - ReservationTxMaxFee: 5000, - MaxReservationsPerWallet: 5, - } - ralc.maxPerWalletAmount = 5000000 - ralc.maxSingleAmount = 5000000 - ralc.maxActive = 100 - - ralc.SetDepositMinAge(3600) - ralc.SetWallet( - walletPublicKeyHash, - &tbtc.WalletChainData{State: tbtc.StateLive}, - ) - currentBlock := uint64(300000) - blockCounter := tbtcpg.NewMockBlockCounter() - blockCounter.SetCurrentBlock(currentBlock) - ralc.SetBlockCounter(blockCounter) + + ralc := newBoundaryTestChain(t, walletPublicKeyHash, currentBlock, nil) fundingTxHash := hashFromString( "7777777777777777777777777777777777777777777777777777777777777777", @@ -1831,9 +1777,7 @@ func TestReservationAcceptanceTask_Stateless_PastEventsError(t *testing.T) { Amount: 2000000, RevealedAt: time.Now().Add(-2 * time.Hour), SweptAt: time.Unix(0, 0), - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], + Vault: &[]chain.Address{testReservationVaultAddress}[0], }, ) @@ -1853,9 +1797,7 @@ func TestReservationAcceptanceTask_Stateless_PastEventsError(t *testing.T) { WalletPublicKeyHash: walletPublicKeyHash, FundingTxHash: fundingTxHash, FundingOutputIndex: 0, - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], + Vault: &[]chain.Address{testReservationVaultAddress}[0], }, ); err != nil { t.Fatal(err) @@ -1891,35 +1833,14 @@ func TestReservationAcceptanceTask_Stateless_NonEligibleReservationState(t *test for _, state := range nonEligibleStates { t.Run(fmt.Sprintf("state_%v", state), func(t *testing.T) { - ralc := newReservationAcceptanceLocalChain() btcChain := tbtcpg.NewLocalBitcoinChain() walletPublicKeyHash := hexToByte20( "8db50eb52063ea9d98b3eac91489a90f738986f6", ) - - ralc.reservationParameters = &tbtc.ReservationParameters{ - ReservationVault: chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - ), - ReservationMinAmount: 1000, - ReservationTxMaxFee: 5000, - MaxReservationsPerWallet: 5, - } - ralc.maxPerWalletAmount = 5000000 - ralc.maxSingleAmount = 5000000 - ralc.maxActive = 100 - - ralc.SetDepositMinAge(3600) - ralc.SetWallet( - walletPublicKeyHash, - &tbtc.WalletChainData{State: tbtc.StateLive}, - ) - currentBlock := uint64(300000) - blockCounter := tbtcpg.NewMockBlockCounter() - blockCounter.SetCurrentBlock(currentBlock) - ralc.SetBlockCounter(blockCounter) + + ralc := newBoundaryTestChain(t, walletPublicKeyHash, currentBlock, nil) fundingTxHash := hashFromString( "8888888888888888888888888888888888888888888888888888888888888888", @@ -1944,9 +1865,7 @@ func TestReservationAcceptanceTask_Stateless_NonEligibleReservationState(t *test Amount: 2000000, RevealedAt: time.Now().Add(-2 * time.Hour), SweptAt: time.Unix(0, 0), - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], + Vault: &[]chain.Address{testReservationVaultAddress}[0], }, ) @@ -1966,9 +1885,7 @@ func TestReservationAcceptanceTask_Stateless_NonEligibleReservationState(t *test WalletPublicKeyHash: walletPublicKeyHash, FundingTxHash: fundingTxHash, FundingOutputIndex: 0, - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], + Vault: &[]chain.Address{testReservationVaultAddress}[0], }, ); err != nil { t.Fatal(err) @@ -2001,36 +1918,24 @@ func TestReservationAcceptanceTask_Stateless_NonEligibleReservationState(t *test // but when governance lowers the minimum amount, Run 2 on the same task instance // proposes it. func TestReservationAcceptanceTask_Stateless_DynamicMinAmount(t *testing.T) { - ralc := newReservationAcceptanceLocalChain() btcChain := tbtcpg.NewLocalBitcoinChain() walletPublicKeyHash := hexToByte20( "8db50eb52063ea9d98b3eac91489a90f738986f6", ) + currentBlock := uint64(300000) // Initial min amount is 5,000,000. - ralc.reservationParameters = &tbtc.ReservationParameters{ - ReservationVault: chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - ), - ReservationMinAmount: 5000000, - ReservationTxMaxFee: 5000, - MaxReservationsPerWallet: 5, - } - ralc.maxPerWalletAmount = 50000000 - ralc.maxSingleAmount = 50000000 - ralc.maxActive = 100 - - ralc.SetDepositMinAge(3600) - ralc.SetWallet( - walletPublicKeyHash, - &tbtc.WalletChainData{State: tbtc.StateLive}, - ) - - currentBlock := uint64(300000) - blockCounter := tbtcpg.NewMockBlockCounter() - blockCounter.SetCurrentBlock(currentBlock) - ralc.SetBlockCounter(blockCounter) + ralc := newBoundaryTestChain(t, walletPublicKeyHash, currentBlock, func(ralc *reservationAcceptanceLocalChain) { + ralc.SetReservationParameters(tbtc.ReservationParameters{ + ReservationVault: testReservationVaultAddress, + ReservationMinAmount: 5000000, + ReservationTxMaxFee: 5000, + MaxReservationsPerWallet: 5, + }) + ralc.maxPerWalletAmount = 50000000 + ralc.maxSingleAmount = 50000000 + }) fundingTxHash := hashFromString( "9999999999999999999999999999999999999999999999999999999999999999", @@ -2057,9 +1962,7 @@ func TestReservationAcceptanceTask_Stateless_DynamicMinAmount(t *testing.T) { Amount: 2000000, RevealedAt: time.Now().Add(-2 * time.Hour), SweptAt: time.Unix(0, 0), - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], + Vault: &[]chain.Address{testReservationVaultAddress}[0], }, ) @@ -2079,9 +1982,7 @@ func TestReservationAcceptanceTask_Stateless_DynamicMinAmount(t *testing.T) { WalletPublicKeyHash: walletPublicKeyHash, FundingTxHash: fundingTxHash, FundingOutputIndex: 0, - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], + Vault: &[]chain.Address{testReservationVaultAddress}[0], }, ); err != nil { t.Fatal(err) @@ -2102,14 +2003,12 @@ func TestReservationAcceptanceTask_Stateless_DynamicMinAmount(t *testing.T) { } // Governance lowers min amount to 1,000,000. - ralc.reservationParameters = &tbtc.ReservationParameters{ - ReservationVault: chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - ), + ralc.SetReservationParameters(tbtc.ReservationParameters{ + ReservationVault: testReservationVaultAddress, ReservationMinAmount: 1000000, ReservationTxMaxFee: 5000, MaxReservationsPerWallet: 5, - } + }) // Second run on the same task instance: deposit is now above min and proposed. proposal, shouldExecute, err = task.Run(request) @@ -2125,35 +2024,14 @@ func TestReservationAcceptanceTask_Stateless_DynamicMinAmount(t *testing.T) { // when an existing reservation record has RequestNonce = N, the generated proposal // uses RequestNonce = N + 1. func TestReservationAcceptanceTask_Stateless_RequestNonceIncremented(t *testing.T) { - ralc := newReservationAcceptanceLocalChain() btcChain := tbtcpg.NewLocalBitcoinChain() walletPublicKeyHash := hexToByte20( "8db50eb52063ea9d98b3eac91489a90f738986f6", ) - - ralc.reservationParameters = &tbtc.ReservationParameters{ - ReservationVault: chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - ), - ReservationMinAmount: 1000, - ReservationTxMaxFee: 5000, - MaxReservationsPerWallet: 5, - } - ralc.maxPerWalletAmount = 5000000 - ralc.maxSingleAmount = 5000000 - ralc.maxActive = 100 - - ralc.SetDepositMinAge(3600) - ralc.SetWallet( - walletPublicKeyHash, - &tbtc.WalletChainData{State: tbtc.StateLive}, - ) - currentBlock := uint64(300000) - blockCounter := tbtcpg.NewMockBlockCounter() - blockCounter.SetCurrentBlock(currentBlock) - ralc.SetBlockCounter(blockCounter) + + ralc := newBoundaryTestChain(t, walletPublicKeyHash, currentBlock, nil) fundingTxHash := hashFromString( "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", @@ -2179,9 +2057,7 @@ func TestReservationAcceptanceTask_Stateless_RequestNonceIncremented(t *testing. Amount: 2000000, RevealedAt: time.Now().Add(-2 * time.Hour), SweptAt: time.Unix(0, 0), - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], + Vault: &[]chain.Address{testReservationVaultAddress}[0], }, ) @@ -2201,9 +2077,7 @@ func TestReservationAcceptanceTask_Stateless_RequestNonceIncremented(t *testing. WalletPublicKeyHash: walletPublicKeyHash, FundingTxHash: fundingTxHash, FundingOutputIndex: 0, - Vault: &[]chain.Address{chain.Address( - "0xReservationVaultAddress1234567890abcdef12345678", - )}[0], + Vault: &[]chain.Address{testReservationVaultAddress}[0], }, ); err != nil { t.Fatal(err) @@ -2244,3 +2118,83 @@ func TestReservationAcceptanceTask_Stateless_RequestNonceIncremented(t *testing. ) } } + +// TestReservationAcceptanceTask_PastDepositRevealedEventsError verifies that +// a genuine (non-sentinel) error from PastDepositRevealedEvents is +// propagated as a hard error, rather than being swallowed like the mock's +// "no events for given filter" sentinel. +func TestReservationAcceptanceTask_PastDepositRevealedEventsError(t *testing.T) { + btcChain := tbtcpg.NewLocalBitcoinChain() + + walletPublicKeyHash := hexToByte20( + "8db50eb52063ea9d98b3eac91489a90f738986f6", + ) + currentBlock := uint64(300000) + + ralc := newBoundaryTestChain(t, walletPublicKeyHash, currentBlock, nil) + + // Otherwise-eligible deposit; the injected error must still short + // circuit before any candidate is ever evaluated. + setupEligibleDeposit( + t, + ralc, + btcChain, + walletPublicKeyHash, + currentBlock, + 2000000, + ) + + ralc.pastDepositRevealedEventsErr = fmt.Errorf("simulated rpc failure") + + task := tbtcpg.NewReservationAcceptanceTask(ralc, btcChain) + + _, shouldExecute, err := task.Run(&tbtc.CoordinationProposalRequest{ + WalletPublicKeyHash: walletPublicKeyHash, + }) + if err == nil { + t.Fatalf("expected a non-nil error, got nil") + } + if shouldExecute { + t.Errorf("expected shouldExecute=false, got true") + } +} + +// TestReservationAcceptanceTask_ValidateProposalError verifies that a +// ValidateReservationAnchorProposal failure aborts proposal generation with +// a wrapped error, rather than being silently ignored. +func TestReservationAcceptanceTask_ValidateProposalError(t *testing.T) { + btcChain := tbtcpg.NewLocalBitcoinChain() + + walletPublicKeyHash := hexToByte20( + "8db50eb52063ea9d98b3eac91489a90f738986f6", + ) + currentBlock := uint64(300000) + + ralc := newBoundaryTestChain(t, walletPublicKeyHash, currentBlock, nil) + + setupEligibleDeposit( + t, + ralc, + btcChain, + walletPublicKeyHash, + currentBlock, + 2000000, + ) + + ralc.validateErr = fmt.Errorf("simulated validation failure") + + task := tbtcpg.NewReservationAcceptanceTask(ralc, btcChain) + + proposal, shouldExecute, err := task.Run(&tbtc.CoordinationProposalRequest{ + WalletPublicKeyHash: walletPublicKeyHash, + }) + if err == nil { + t.Fatalf("expected a non-nil error, got nil") + } + if shouldExecute { + t.Errorf("expected shouldExecute=false, got true") + } + if proposal != nil { + t.Errorf("expected nil proposal, got %v", proposal) + } +} From edd054e7c1f69ad792088a4a946ce4448098bc88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ros=C5=82aniec?= Date: Thu, 3 Sep 2026 16:11:52 +0000 Subject: [PATCH 9/9] test(tbtcpg): assert genuine pending-action state in dedup test The 'records acceptance request, skipping duplicate on subsequent run' test relied on hasPendingAction's fail-closed default on a missing GetReservationAction record, not on the intended pending-state detection path - its comment still described the removed PastReservationAcceptanceRequestedEvents mechanism. Explicitly set the action record to Pending after the first run so the second run's dedup assertion genuinely exercises hasPendingAction's happy path. --- pkg/tbtcpg/reservation_acceptance_test.go | 25 +++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/pkg/tbtcpg/reservation_acceptance_test.go b/pkg/tbtcpg/reservation_acceptance_test.go index 7c7b47690a..fa7ac1c9ff 100644 --- a/pkg/tbtcpg/reservation_acceptance_test.go +++ b/pkg/tbtcpg/reservation_acceptance_test.go @@ -1529,7 +1529,7 @@ func TestReservationAcceptanceTask_ReservationParametersFetchedLive(t *testing.T ralc := newBoundaryTestChain(t, walletPublicKeyHash, currentBlock, nil) - setupEligibleDeposit( + fundingTxHash := setupEligibleDeposit( t, ralc, btcChain, @@ -1552,18 +1552,27 @@ func TestReservationAcceptanceTask_ReservationParametersFetchedLive(t *testing.T t.Fatalf("expected shouldExecute=true on first run, got false") } - // Second run on the same, now-requested deposit: RequestReservationAcceptance - // recorded a ReservationAcceptanceRequestedEvent as a side effect of the - // first run, so production's dedup guard (which queries - // PastReservationAcceptanceRequestedEvents) must now find it and skip the - // candidate. Without the fixture actually recording that event, this run - // would (incorrectly) accept again and the dedup guard would go untested. + // RequestReservationAcceptance bumped RequestNonce to 1 as a side + // effect of the first run, mirroring the on-chain Bridge. On real + // chain the Bridge also marks that generation's action record + // Pending; record that here too so the second run's dedup guard + // (hasPendingAction, which reads GetReservationAction) genuinely + // observes a pending generation instead of merely fail-closing on + // a not-found lookup. + depositKey := ralc.BuildDepositKey(fundingTxHash, 0) + ralc.SetReservationAction(depositKey, 1, &tbtc.ReservationAction{ + State: tbtc.ReservationActionStatePending, + }) + + // Second run on the same, now-requested deposit: the pending + // action generation recorded above must be found and the + // candidate skipped. _, shouldExecute, err = task.Run(request) if err != nil { t.Fatalf("unexpected error on second run: [%v]", err) } if shouldExecute { - t.Fatalf("expected shouldExecute=false on second run due to existing acceptance request, got true") + t.Fatalf("expected shouldExecute=false on second run due to pending acceptance action, got true") } })