diff --git a/.github/workflows/reviewdog.yml b/.github/workflows/reviewdog.yml index 5a79d3709..62afe323e 100644 --- a/.github/workflows/reviewdog.yml +++ b/.github/workflows/reviewdog.yml @@ -20,6 +20,20 @@ jobs: fail_on_error: true pattern: TODO_IN_THIS_ + check_non_standard_interface_implementations: + name: Check for non-standard interface implementation statements + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: pokt-network/action-fail-on-found@v1 + with: + github_token: ${{ secrets.github_token }} + reporter: github-pr-review + level: error + fail_on_error: true + pattern: var _ .* = &.*{} + ignore: .github,.git + # More info: https://github.com/reviewdog/action-misspell check_misspell: name: Check misspelling @@ -31,4 +45,4 @@ jobs: github_token: ${{ secrets.github_token }} reporter: github-check level: warning - locale: "US" \ No newline at end of file + locale: "US" diff --git a/Makefile b/Makefile index c13061153..9873e1206 100644 --- a/Makefile +++ b/Makefile @@ -123,7 +123,10 @@ go_mockgen: ## Use `mockgen` to generate mocks used for testing purposes of all go generate ./x/supplier/types/ .PHONY: go_develop -go_develop: proto_regen go_mockgen go_test ## Generate protos, mocks and run all tests +go_develop: proto_regen go_mockgen ## Generate protos and mocks + +.PHONY: go_develop_and_test +go_develop_and_test: go_develop go_test ## Generate protos, mocks and run all tests ############# ### TODOS ### diff --git a/docs/pkg/observable/README.md b/docs/pkg/observable/README.md index 9523ff457..3234b627b 100644 --- a/docs/pkg/observable/README.md +++ b/docs/pkg/observable/README.md @@ -1,6 +1,6 @@ -## `pkg/observable` Package +## `pocket/pkg/observable` Package -The `pkg/observable` package provides a lightweight and straightforward mechanism to handle asynchronous notifications using the Observer pattern. This is achieved through two primary interfaces: `Observable` and `Observer`. +The `pocket/pkg/observable` package provides a lightweight and straightforward mechanism to handle asynchronous notifications using the Observer pattern. This is achieved through two primary interfaces: `Observable` and `Observer`. ## Overview diff --git a/docs/template/pkg/README.md b/docs/template/pkg/README.md index c7f915e9a..44f41885a 100644 --- a/docs/template/pkg/README.md +++ b/docs/template/pkg/README.md @@ -1,6 +1,3 @@ -Certainly! I've added a section named "Architecture Diagrams" in the documentation template below: - -```markdown # Package [PackageName] > Brief one-liner or quote about what this package does. @@ -16,11 +13,21 @@ Provide a few sentences about the purpose and functionality of this package. Con Visual representations often make it easier to understand the design and flow of a package. Below are the architecture diagrams that explain the high-level structure and interactions in this package: -![Architecture Overview](./path-to-diagram1.png) +```mermaid +--- +title: Architecture Overview +--- +flowchart +``` > **Figure 1**: Brief description about what this diagram represents. -![Another Diagram](./path-to-diagram2.png) +```mermaid +--- +title: Another Diagram +--- +flowchart +``` > **Figure 2**: Brief description about what this other diagram represents. diff --git a/e2e/tests/init_test.go b/e2e/tests/init_test.go index 0ab3be752..fe831a507 100644 --- a/e2e/tests/init_test.go +++ b/e2e/tests/init_test.go @@ -5,26 +5,37 @@ package e2e import ( "fmt" "regexp" + "strconv" "strings" "testing" + "time" "github.com/regen-network/gocuke" "github.com/stretchr/testify/require" ) -var addrRe *regexp.Regexp +var ( + addrRe *regexp.Regexp + amountRe *regexp.Regexp + accNameToAddrMap = make(map[string]string) + keyRingFlag = "--keyring-backend=test" +) func init() { - addrRe = regexp.MustCompile(`address:\s+(pokt1\w+)`) + addrRe = regexp.MustCompile(`address: (\S+)\s+name: (\S+)`) + amountRe = regexp.MustCompile(`amount: "(.+?)"\s+denom: upokt`) } type suite struct { gocuke.TestingT - pocketd *pocketdBin + pocketd *pocketdBin + scenarioState map[string]any // temporary state for each scenario } func (s *suite) Before() { s.pocketd = new(pocketdBin) + s.scenarioState = make(map[string]any) + s.buildAddrMap() } // TestFeatures runs the e2e tests specified in any .features files in this directory @@ -56,17 +67,15 @@ func (s *suite) TheUserShouldBeAbleToSeeStandardOutputContaining(arg1 string) { } } -func (s *suite) TheUserSendsUpoktToAnotherAddress(amount int64) { - addrs := s.getAddresses() +func (s *suite) TheUserSendsUpoktFromAccountToAccount(amount int64, accName1, accName2 string) { args := []string{ "tx", "bank", "send", - addrs[0], - addrs[1], + accNameToAddrMap[accName1], + accNameToAddrMap[accName2], fmt.Sprintf("%dupokt", amount), - "--keyring-backend", - "test", + keyRingFlag, "-y", } res, err := s.pocketd.RunCommandOnHost("", args...) @@ -76,20 +85,78 @@ func (s *suite) TheUserSendsUpoktToAnotherAddress(amount int64) { s.pocketd.result = res } -func (s *suite) getAddresses() [2]string { - var strs [2]string +func (s *suite) TheAccountHasABalanceGreaterThanUpokt(accName string, amount int64) { + bal := s.getAccBalance(accName) + if int64(bal) < amount { + s.Fatalf("account %s does not have enough upokt: %d < %d", accName, bal, amount) + } + s.scenarioState[accName] = bal // save the balance for later +} + +func (s *suite) AnAccountExistsFor(accName string) { + bal := s.getAccBalance(accName) + s.scenarioState[accName] = bal // save the balance for later +} + +func (s *suite) TheAccountBalanceOfShouldBeUpoktThanBefore(accName string, amount int64, condition string) { + prev, ok := s.scenarioState[accName] + if !ok { + s.Fatalf("no previous balance found for %s", accName) + } + + bal := s.getAccBalance(accName) + switch condition { + case "more": + if bal <= prev.(int) { + s.Fatalf("account %s expected to have more upokt but: %d <= %d", accName, bal, prev) + } + case "less": + if bal >= prev.(int) { + s.Fatalf("account %s expected to have less upokt but: %d >= %d", accName, bal, prev) + } + default: + s.Fatalf("unknown condition %s", condition) + } +} + +func (s *suite) TheUserShouldWaitForSeconds(dur int64) { + time.Sleep(time.Duration(dur) * time.Second) +} + +func (s *suite) buildAddrMap() { + s.Helper() res, err := s.pocketd.RunCommand( - "keys", "list", "--keyring-backend", "test", + "keys", "list", keyRingFlag, ) if err != nil { s.Fatalf("error getting keys: %s", err) } matches := addrRe.FindAllStringSubmatch(res.Stdout, -1) - if len(matches) >= 2 { - strs[0] = matches[0][1] - strs[1] = matches[len(matches)-1][1] - } else { - s.Fatalf("could not find two addresses in output: %s", res.Stdout) + for _, match := range matches { + name := match[2] + address := match[1] + accNameToAddrMap[name] = address + } +} + +func (s *suite) getAccBalance(accName string) int { + s.Helper() + args := []string{ + "query", + "bank", + "balances", + accNameToAddrMap[accName], + } + res, err := s.pocketd.RunCommandOnHost("", args...) + if err != nil { + s.Fatalf("error getting balance: %s", err) + } + s.pocketd.result = res + match := amountRe.FindStringSubmatch(res.Stdout) + if len(match) < 2 { + s.Fatalf("no balance found for %s", accName) } - return strs + found, err := strconv.Atoi(match[1]) + require.NoError(s, err) + return found } diff --git a/e2e/tests/node.go b/e2e/tests/node.go index 4a025cae5..4e34fa827 100644 --- a/e2e/tests/node.go +++ b/e2e/tests/node.go @@ -24,7 +24,7 @@ func init() { defaultRPCURL = fmt.Sprintf("tcp://%s:%d", defaultRPCHost, defaultRPCPort) } if defaultHome == "" { - defaultHome = "./localnet/pocketd" + defaultHome = "../../localnet/pocketd" } } @@ -42,7 +42,7 @@ type PocketClient interface { } // Ensure that pocketdBin struct fulfills PocketClient -var _ PocketClient = &pocketdBin{} +var _ PocketClient = (*pocketdBin)(nil) // pocketdBin holds the reults of the last command that was run type pocketdBin struct { diff --git a/e2e/tests/send.feature b/e2e/tests/send.feature index 1e2bdf2f8..dc5fc4504 100644 --- a/e2e/tests/send.feature +++ b/e2e/tests/send.feature @@ -2,7 +2,12 @@ Feature: Tx Namespace Scenario: User can send uPOKT Given the user has the pocketd binary installed - When the user sends 10000 uPOKT to another address + And the account "app1" has a balance greater than "1000" uPOKT + And an account exists for "app2" + When the user sends "1000" uPOKT from account "app1" to account "app2" Then the user should be able to see standard output containing "txhash:" And the user should be able to see standard output containing "code: 0" And the pocketd binary should exit without error + And the user should wait for "5" seconds + And the account balance of "app1" should be "1000" uPOKT "less" than before + And the account balance of "app2" should be "1000" uPOKT "more" than before diff --git a/pkg/observable/channel/observable.go b/pkg/observable/channel/observable.go index a173d8072..26958a75b 100644 --- a/pkg/observable/channel/observable.go +++ b/pkg/observable/channel/observable.go @@ -2,8 +2,9 @@ package channel import ( "context" - "pocket/pkg/observable" "sync" + + "pocket/pkg/observable" ) // TODO_DISCUSS: what should this be? should it be configurable? It seems to be most @@ -12,7 +13,7 @@ import ( // defaultSubscribeBufferSize is the buffer size of a observable's publish channel. const defaultPublishBufferSize = 50 -var _ observable.Observable[any] = &channelObservable[any]{} +var _ observable.Observable[any] = (*channelObservable[any])(nil) // option is a function which receives and can modify the channelObservable state. type option[V any] func(obs *channelObservable[V]) @@ -50,7 +51,7 @@ func NewObservable[V any](opts ...option[V]) (observable.Observable[V], chan<- V } // start listening to the publishCh and emit values to observers - go obs.goPublish(obs.publishCh) + go obs.goPublish() return obs, obs.publishCh } @@ -63,6 +64,15 @@ func WithPublisher[V any](publishCh chan V) option[V] { } } +// Next synchronously returns the next value from the observable. +func (obsvbl *channelObservable[V]) Next(ctx context.Context) V { + tempObserver := obsvbl.Subscribe(ctx) + defer tempObserver.Unsubscribe() + + val := <-tempObserver.Ch() + return val +} + // Subscribe returns an observer which is notified when the publishCh channel // receives a value. func (obsvbl *channelObservable[V]) Subscribe(ctx context.Context) observable.Observer[V] { @@ -110,8 +120,8 @@ func (obsvbl *channelObservable[V]) unsubscribeAll() { // goPublish to the publishCh and notify observers when values are received. // This function is blocking and should be run in a goroutine. -func (obsvbl *channelObservable[V]) goPublish(publisher <-chan V) { - for notification := range publisher { +func (obsvbl *channelObservable[V]) goPublish() { + for notification := range obsvbl.publishCh { // Copy currentObservers to avoid holding the lock while notifying them. // New or existing Observers may (un)subscribe while this notification // is being fanned out. @@ -154,9 +164,12 @@ func (obsvbl *channelObservable[V]) copyObservers() (observers []*channelObserve // goUnsubscribeOnDone unsubscribes from the subscription when the context is done. // It is a blocking function and intended to be called in a goroutine. -func goUnsubscribeOnDone[V any](ctx context.Context, subscription observable.Observer[V]) { +func goUnsubscribeOnDone[V any](ctx context.Context, observer observable.Observer[V]) { <-ctx.Done() - subscription.Unsubscribe() + if observer.IsClosed() { + return + } + observer.Unsubscribe() } // onUnsubscribe returns a function that removes a given observer from the diff --git a/pkg/observable/channel/observable_test.go b/pkg/observable/channel/observable_test.go index c6f27929f..6ec301cfa 100644 --- a/pkg/observable/channel/observable_test.go +++ b/pkg/observable/channel/observable_test.go @@ -38,7 +38,7 @@ func TestChannelObservable_NotifyObservers(t *testing.T) { tests := []test{ { - name: "nil publisher", + name: "nil publisher (default buffer size)", publishCh: nil, inputs: inputs, expectedOutputs: inputs, @@ -55,7 +55,13 @@ func TestChannelObservable_NotifyObservers(t *testing.T) { inputs: inputs, expectedOutputs: inputs, }, - // TODO_INCOMPLETE: publisher channels which are full are proving harder to test + { + name: "empty buffered len 1000 publisher", + publishCh: make(chan int, 1000), + inputs: inputs, + expectedOutputs: inputs, + }, + // TODO_INCOMPLETE(#81): publisher channels which are full are proving harder to test // robustly (no flakiness); perhaps it has to do with the lack of some // kind of guarantee about the receiver order on the consumer side. // @@ -156,7 +162,7 @@ func TestChannelObservable_NotifyObservers(t *testing.T) { err := group.Wait() require.NoError(t, err) - // unsubscribing should unsubscribeAll obsvr channel(s) + // unsubscribing should close observer channel(s) for _, observer := range observers { observer.Unsubscribe() @@ -205,6 +211,8 @@ func TestChannelObservable_UnsubscribeObservers(t *testing.T) { }, }, { + // NOTE: this will log a warning that can be ignored: + // > redundant unsubscribe: observer is closed name: "cancel then unsubscribe", lifecycleFn: func() observable.Observer[int] { observer := obsvbl.Subscribe(ctx) @@ -215,6 +223,8 @@ func TestChannelObservable_UnsubscribeObservers(t *testing.T) { }, }, { + // NOTE: this will log a warning that can be ignored: + // > redundant unsubscribe: observer is closed name: "unsubscribe then cancel", lifecycleFn: func() observable.Observer[int] { observer := obsvbl.Subscribe(ctx) diff --git a/pkg/observable/channel/observer.go b/pkg/observable/channel/observer.go index 7bd7ddbd3..394a6bdbb 100644 --- a/pkg/observable/channel/observer.go +++ b/pkg/observable/channel/observer.go @@ -24,7 +24,7 @@ const ( sendRetryInterval = 100 * time.Millisecond ) -var _ observable.Observer[any] = &channelObserver[any]{} +var _ observable.Observer[any] = (*channelObserver[any])(nil) // channelObserver implements the observable.Observer interface. type channelObserver[V any] struct { @@ -69,6 +69,15 @@ func (obsvr *channelObserver[V]) Ch() <-chan V { return obsvr.observerCh } +// IsClosed returns true if the observer has been unsubscribed. +// A closed observer cannot be reused. +func (obsvr *channelObserver[V]) IsClosed() bool { + obsvr.observerMu.Lock() + defer obsvr.observerMu.Unlock() + + return obsvr.isClosed +} + // unsubscribe closes the subscription channel, marks the observer as isClosed, and // removes the subscription from its observable's observers list via onUnsubscribe. func (obsvr *channelObserver[V]) unsubscribe() { diff --git a/pkg/observable/interface.go b/pkg/observable/interface.go index 3d4894339..452c18dcd 100644 --- a/pkg/observable/interface.go +++ b/pkg/observable/interface.go @@ -11,7 +11,12 @@ import "context" // notified of new values asynchronously. // It is analogous to a publisher in a "Fan-Out" system design. type Observable[V any] interface { + // Next synchronously returns the next value from the observable. + Next(context.Context) V + // Subscribe returns an observer which is notified when the publishCh channel + // receives a value. Subscribe(context.Context) Observer[V] + // UnsubscribeAll unsubscribes and removes all observers from the observable. UnsubscribeAll() } @@ -19,6 +24,12 @@ type Observable[V any] interface { // channel and allows unsubscribing from an Observable. // It is analogous to a subscriber in a "Fan-Out" system design. type Observer[V any] interface { + // Unsubscribe closes the subscription channel and removes the subscription from + // the observable. Unsubscribe() + // Ch returns a receive-only subscription channel. Ch() <-chan V + // IsClosed returns true if the observer has been unsubscribed. + // A closed observer cannot be reused. + IsClosed() bool } diff --git a/pkg/relayer/proxy/proxy.go b/pkg/relayer/proxy/proxy.go index f3d53f5e0..f626f184f 100644 --- a/pkg/relayer/proxy/proxy.go +++ b/pkg/relayer/proxy/proxy.go @@ -16,7 +16,7 @@ import ( suppliertypes "pocket/x/supplier/types" ) -var _ RelayerProxy = &relayerProxy{} +var _ RelayerProxy = (*relayerProxy)(nil) type relayerProxy struct { // keyName is the supplier's key name in the Cosmos's keybase. It is used along with the keyring to diff --git a/x/application/types/message_delegate_to_gateway.go b/x/application/types/message_delegate_to_gateway.go index d162c61bf..232564310 100644 --- a/x/application/types/message_delegate_to_gateway.go +++ b/x/application/types/message_delegate_to_gateway.go @@ -7,7 +7,7 @@ import ( const TypeMsgDelegateToGateway = "delegate_to_gateway" -var _ sdk.Msg = &MsgDelegateToGateway{} +var _ sdk.Msg = (*MsgDelegateToGateway)(nil) func NewMsgDelegateToGateway(address string) *MsgDelegateToGateway { return &MsgDelegateToGateway{ diff --git a/x/application/types/message_stake_application.go b/x/application/types/message_stake_application.go index 93532f2bd..219a65701 100644 --- a/x/application/types/message_stake_application.go +++ b/x/application/types/message_stake_application.go @@ -8,7 +8,7 @@ import ( const TypeMsgStakeApplication = "stake_application" -var _ sdk.Msg = &MsgStakeApplication{} +var _ sdk.Msg = (*MsgStakeApplication)(nil) func NewMsgStakeApplication( address string, diff --git a/x/application/types/message_undelegate_from_gateway.go b/x/application/types/message_undelegate_from_gateway.go index 2cca7962c..240605383 100644 --- a/x/application/types/message_undelegate_from_gateway.go +++ b/x/application/types/message_undelegate_from_gateway.go @@ -7,7 +7,7 @@ import ( const TypeMsgUndelegateFromGateway = "undelegate_from_gateway" -var _ sdk.Msg = &MsgUndelegateFromGateway{} +var _ sdk.Msg = (*MsgUndelegateFromGateway)(nil) func NewMsgUndelegateFromGateway(address string) *MsgUndelegateFromGateway { return &MsgUndelegateFromGateway{ diff --git a/x/application/types/message_unstake_application.go b/x/application/types/message_unstake_application.go index 1f4051f52..010bae551 100644 --- a/x/application/types/message_unstake_application.go +++ b/x/application/types/message_unstake_application.go @@ -7,7 +7,7 @@ import ( const TypeMsgUnstakeApplication = "unstake_application" -var _ sdk.Msg = &MsgUnstakeApplication{} +var _ sdk.Msg = (*MsgUnstakeApplication)(nil) func NewMsgUnstakeApplication(address string) *MsgUnstakeApplication { return &MsgUnstakeApplication{ diff --git a/x/gateway/types/message_stake_gateway.go b/x/gateway/types/message_stake_gateway.go index c86375288..70811076c 100644 --- a/x/gateway/types/message_stake_gateway.go +++ b/x/gateway/types/message_stake_gateway.go @@ -8,7 +8,7 @@ import ( const TypeMsgStakeGateway = "stake_gateway" -var _ sdk.Msg = &MsgStakeGateway{} +var _ sdk.Msg = (*MsgStakeGateway)(nil) func NewMsgStakeGateway(address string, stake types.Coin) *MsgStakeGateway { return &MsgStakeGateway{ diff --git a/x/gateway/types/message_unstake_gateway.go b/x/gateway/types/message_unstake_gateway.go index cb60f2009..23bf5f97a 100644 --- a/x/gateway/types/message_unstake_gateway.go +++ b/x/gateway/types/message_unstake_gateway.go @@ -7,7 +7,7 @@ import ( const TypeMsgUnstakeGateway = "unstake_gateway" -var _ sdk.Msg = &MsgUnstakeGateway{} +var _ sdk.Msg = (*MsgUnstakeGateway)(nil) func NewMsgUnstakeGateway(address string) *MsgUnstakeGateway { return &MsgUnstakeGateway{ diff --git a/x/supplier/types/message_create_claim.go b/x/supplier/types/message_create_claim.go index 98acf0c72..4bcfada3b 100644 --- a/x/supplier/types/message_create_claim.go +++ b/x/supplier/types/message_create_claim.go @@ -9,7 +9,7 @@ import ( const TypeMsgCreateClaim = "create_claim" -var _ sdk.Msg = &MsgCreateClaim{} +var _ sdk.Msg = (*MsgCreateClaim)(nil) func NewMsgCreateClaim(supplierAddress string, sessionHeader *sessiontypes.SessionHeader, rootHash []byte) *MsgCreateClaim { return &MsgCreateClaim{ diff --git a/x/supplier/types/message_stake_supplier.go b/x/supplier/types/message_stake_supplier.go index a441ce6bc..9d4ce5cdc 100644 --- a/x/supplier/types/message_stake_supplier.go +++ b/x/supplier/types/message_stake_supplier.go @@ -8,7 +8,7 @@ import ( const TypeMsgStakeSupplier = "stake_supplier" -var _ sdk.Msg = &MsgStakeSupplier{} +var _ sdk.Msg = (*MsgStakeSupplier)(nil) func NewMsgStakeSupplier( address string, diff --git a/x/supplier/types/message_submit_proof.go b/x/supplier/types/message_submit_proof.go index ee2dec47a..32faa796b 100644 --- a/x/supplier/types/message_submit_proof.go +++ b/x/supplier/types/message_submit_proof.go @@ -9,7 +9,7 @@ import ( const TypeMsgSubmitProof = "submit_proof" -var _ sdk.Msg = &MsgSubmitProof{} +var _ sdk.Msg = (*MsgSubmitProof)(nil) func NewMsgSubmitProof(supplierAddress string, sessionHeader *sessiontypes.SessionHeader, proof []byte) *MsgSubmitProof { return &MsgSubmitProof{ diff --git a/x/supplier/types/message_unstake_supplier.go b/x/supplier/types/message_unstake_supplier.go index eec8ede80..884c21c36 100644 --- a/x/supplier/types/message_unstake_supplier.go +++ b/x/supplier/types/message_unstake_supplier.go @@ -7,7 +7,7 @@ import ( const TypeMsgUnstakeSupplier = "unstake_supplier" -var _ sdk.Msg = &MsgUnstakeSupplier{} +var _ sdk.Msg = (*MsgUnstakeSupplier)(nil) func NewMsgUnstakeSupplier(address string) *MsgUnstakeSupplier { return &MsgUnstakeSupplier{