From d53c5e25e450b2e002f354901c6b33347258408d Mon Sep 17 00:00:00 2001 From: srdtrk Date: Wed, 16 Aug 2023 13:56:33 +0300 Subject: [PATCH 1/5] docs(callbacks): added godocs for send packet validation --- modules/apps/callbacks/types/expected_keepers.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/modules/apps/callbacks/types/expected_keepers.go b/modules/apps/callbacks/types/expected_keepers.go index f21d3dbd923..e91129bf0cf 100644 --- a/modules/apps/callbacks/types/expected_keepers.go +++ b/modules/apps/callbacks/types/expected_keepers.go @@ -31,6 +31,8 @@ type ContractKeeper interface { // the sender is unknown or undefined. The contract is expected to handle the callback within the // user defined gas limit, and handle any errors, or panics gracefully. // If an error is returned, state will be reverted by the callbacks middleware. + // NOTE: Performing the SendPacket validation in this acknowledgement callback is recommended in + // case the callbacks middleware is not wired properly. IBCOnAcknowledgementPacketCallback( ctx sdk.Context, packet channeltypes.Packet, @@ -44,6 +46,8 @@ type ContractKeeper interface { // empty if the sender is unknown or undefined. The contract is expected to handle the callback // within the user defined gas limit, and handle any error, out of gas, or panics gracefully. // If an error is returned, state will be reverted by the callbacks middleware. + // NOTE: Performing the SendPacket validation in this timeout callback is recommended in case the + // callbacks middleware is not wired properly. IBCOnTimeoutPacketCallback( ctx sdk.Context, packet channeltypes.Packet, From 5e43a5908ed171057d4e1e7507492ad67833dcb3 Mon Sep 17 00:00:00 2001 From: srdtrk Date: Wed, 16 Aug 2023 13:58:24 +0300 Subject: [PATCH 2/5] docs(callbacks): improved godocs --- modules/apps/callbacks/types/expected_keepers.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/apps/callbacks/types/expected_keepers.go b/modules/apps/callbacks/types/expected_keepers.go index e91129bf0cf..8f8b55c4780 100644 --- a/modules/apps/callbacks/types/expected_keepers.go +++ b/modules/apps/callbacks/types/expected_keepers.go @@ -31,8 +31,8 @@ type ContractKeeper interface { // the sender is unknown or undefined. The contract is expected to handle the callback within the // user defined gas limit, and handle any errors, or panics gracefully. // If an error is returned, state will be reverted by the callbacks middleware. - // NOTE: Performing the SendPacket validation in this acknowledgement callback is recommended in - // case the callbacks middleware is not wired properly. + // NOTE: Performing the SendPacket validation defensively in this acknowledgement callback is + // recommended in case the callbacks middleware is not wired properly. IBCOnAcknowledgementPacketCallback( ctx sdk.Context, packet channeltypes.Packet, @@ -46,8 +46,8 @@ type ContractKeeper interface { // empty if the sender is unknown or undefined. The contract is expected to handle the callback // within the user defined gas limit, and handle any error, out of gas, or panics gracefully. // If an error is returned, state will be reverted by the callbacks middleware. - // NOTE: Performing the SendPacket validation in this timeout callback is recommended in case the - // callbacks middleware is not wired properly. + // NOTE: Performing the SendPacket validation defensively in this timeout callback is recommended + // in case the callbacks middleware is not wired properly. IBCOnTimeoutPacketCallback( ctx sdk.Context, packet channeltypes.Packet, From f7972ae50a19a460e045d7950f73c70e9be4de04 Mon Sep 17 00:00:00 2001 From: srdtrk Date: Tue, 22 Aug 2023 19:33:49 +0300 Subject: [PATCH 3/5] docs(callbacks): used colin and damian suggested doc string --- .../apps/callbacks/types/expected_keepers.go | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/modules/apps/callbacks/types/expected_keepers.go b/modules/apps/callbacks/types/expected_keepers.go index 8f8b55c4780..3ef2d327db4 100644 --- a/modules/apps/callbacks/types/expected_keepers.go +++ b/modules/apps/callbacks/types/expected_keepers.go @@ -16,6 +16,11 @@ type ContractKeeper interface { // gas limit, and handle any errors, or panics gracefully. // If an error is returned, the transaction will be reverted by the callbacks middleware, and the // packet will not be sent. + // + // Implementations are provided with the packetSendAddress and MAY choose to use this to perform + // validation on the origin of a given packet. It is recommended to perform the same validation + // on all source chain callbacks (SendPacket, AcknowledgementPacket, TimeoutPacket). This + // defensively guards against exploits due to incorrectly wired SendPacket ordering in IBC stacks. IBCSendPacketCallback( ctx sdk.Context, sourcePort string, @@ -31,8 +36,11 @@ type ContractKeeper interface { // the sender is unknown or undefined. The contract is expected to handle the callback within the // user defined gas limit, and handle any errors, or panics gracefully. // If an error is returned, state will be reverted by the callbacks middleware. - // NOTE: Performing the SendPacket validation defensively in this acknowledgement callback is - // recommended in case the callbacks middleware is not wired properly. + // + // Implementations are provided with the packetSendAddress and MAY choose to use this to perform + // validation on the origin of a given packet. It is recommended to perform the same validation + // on all source chain callbacks (SendPacket, AcknowledgementPacket, TimeoutPacket). This + // defensively guards against exploits due to incorrectly wired SendPacket ordering in IBC stacks. IBCOnAcknowledgementPacketCallback( ctx sdk.Context, packet channeltypes.Packet, @@ -46,8 +54,12 @@ type ContractKeeper interface { // empty if the sender is unknown or undefined. The contract is expected to handle the callback // within the user defined gas limit, and handle any error, out of gas, or panics gracefully. // If an error is returned, state will be reverted by the callbacks middleware. - // NOTE: Performing the SendPacket validation defensively in this timeout callback is recommended - // in case the callbacks middleware is not wired properly. + // + // + // Implementations are provided with the packetSendAddress and MAY choose to use this to perform + // validation on the origin of a given packet. It is recommended to perform the same validation + // on all source chain callbacks (SendPacket, AcknowledgementPacket, TimeoutPacket). This + // defensively guards against exploits due to incorrectly wired SendPacket ordering in IBC stacks. IBCOnTimeoutPacketCallback( ctx sdk.Context, packet channeltypes.Packet, From 58b40cde46295b238110d0a73281eb2119a1bbff Mon Sep 17 00:00:00 2001 From: srdtrk Date: Tue, 22 Aug 2023 19:37:05 +0300 Subject: [PATCH 4/5] docs(callbacks): fixed typo --- modules/apps/callbacks/types/expected_keepers.go | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/apps/callbacks/types/expected_keepers.go b/modules/apps/callbacks/types/expected_keepers.go index 3ef2d327db4..e8cd3d52c3a 100644 --- a/modules/apps/callbacks/types/expected_keepers.go +++ b/modules/apps/callbacks/types/expected_keepers.go @@ -55,7 +55,6 @@ type ContractKeeper interface { // within the user defined gas limit, and handle any error, out of gas, or panics gracefully. // If an error is returned, state will be reverted by the callbacks middleware. // - // // Implementations are provided with the packetSendAddress and MAY choose to use this to perform // validation on the origin of a given packet. It is recommended to perform the same validation // on all source chain callbacks (SendPacket, AcknowledgementPacket, TimeoutPacket). This From 0e1aebdf748e181603294d02d8e65b5a2663d88f Mon Sep 17 00:00:00 2001 From: srdtrk Date: Wed, 23 Aug 2023 12:27:48 +0300 Subject: [PATCH 5/5] docs(callbacks): improved a minor typo --- modules/apps/callbacks/types/expected_keepers.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/apps/callbacks/types/expected_keepers.go b/modules/apps/callbacks/types/expected_keepers.go index e8cd3d52c3a..1fc9494c352 100644 --- a/modules/apps/callbacks/types/expected_keepers.go +++ b/modules/apps/callbacks/types/expected_keepers.go @@ -17,7 +17,7 @@ type ContractKeeper interface { // If an error is returned, the transaction will be reverted by the callbacks middleware, and the // packet will not be sent. // - // Implementations are provided with the packetSendAddress and MAY choose to use this to perform + // Implementations are provided with the packetSenderAddress and MAY choose to use this to perform // validation on the origin of a given packet. It is recommended to perform the same validation // on all source chain callbacks (SendPacket, AcknowledgementPacket, TimeoutPacket). This // defensively guards against exploits due to incorrectly wired SendPacket ordering in IBC stacks. @@ -37,7 +37,7 @@ type ContractKeeper interface { // user defined gas limit, and handle any errors, or panics gracefully. // If an error is returned, state will be reverted by the callbacks middleware. // - // Implementations are provided with the packetSendAddress and MAY choose to use this to perform + // Implementations are provided with the packetSenderAddress and MAY choose to use this to perform // validation on the origin of a given packet. It is recommended to perform the same validation // on all source chain callbacks (SendPacket, AcknowledgementPacket, TimeoutPacket). This // defensively guards against exploits due to incorrectly wired SendPacket ordering in IBC stacks. @@ -55,7 +55,7 @@ type ContractKeeper interface { // within the user defined gas limit, and handle any error, out of gas, or panics gracefully. // If an error is returned, state will be reverted by the callbacks middleware. // - // Implementations are provided with the packetSendAddress and MAY choose to use this to perform + // Implementations are provided with the packetSenderAddress and MAY choose to use this to perform // validation on the origin of a given packet. It is recommended to perform the same validation // on all source chain callbacks (SendPacket, AcknowledgementPacket, TimeoutPacket). This // defensively guards against exploits due to incorrectly wired SendPacket ordering in IBC stacks.