Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Paima events #390

Merged
merged 22 commits into from
Jul 28, 2024
Merged

Paima events #390

merged 22 commits into from
Jul 28, 2024

Conversation

acedward
Copy link
Contributor

@acedward acedward commented Jul 1, 2024

New packages

@paima/events

This PR introduces an event system and also sets up the events to use typebox so that it is both strongly typed and so that we can later export them as json-schema for documentation purposes and so that it works well with the json-schema we'll use for the log system

  • doesn't solve the ENV issue
  • disable external users from sending messages to the engine/batcher (I believe this can be done with this)
  • add more status options to the batcher event
  • don't only send block events when a block is non-empty (non-empty should maybe just be a filter on blocks?)
  • fix the issue where block events may be skipped if paima engine shuts down (see Handling shutdowns in event listeners in Emit event standard for games #373)
  • Disable MQTT reserved keywords (/, +, #) in event names
  • test this PR

Define an event

const QuestCompletionEvent = genEvent({
  name: 'QuestCompletion',
  fields: [
    {
      name: 'questId',
      type: Type.Integer(),
      indexed: true,
    },
    {
      name: 'playerId',
      type: Type.Integer(),
    },
  ],
} as const);

Listen to Messages

Listen to an event

PaimaEventManager.Instance.subscribe(
  {
    topic: QuestCompletionEvent,
    filter: { questId: undefined }, // all quests
  },
  event => {
    console.log(`Quest ${event.questId} cleared by ${event.playerId}`);
  }
);

Send Messages

Sent an event

PaimaEventListener.Instance.sendMessage(
  QuestCompletionEvent,
  {
    questId: 5,
    playerId: 10
  }
);

@paima/broker

Start MQTT Server

new PaimaBroker(ENV).getServer('Paima-Engine');

This creates a WebSocket MQTT server

Tarochi Example (with Batcher)

Screen.Recording.2024-07-15.at.12.30.55.mp4

Card-Game (Direct TX)

Screen.Recording.2024-07-15.at.17.20.15.mp4

Overview

image

@@ -95,16 +96,18 @@ class BatchedTransactionPoster {
value: '0x' + Number(this.fee).toString(16),
gasLimit: estimateGasLimit(msg.length),
});
const receipt = (await transaction.extra.wait())!;
// TODO ONLY ACTIVATE IN ASYNC MODE!!!1!
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is something I'm still looking into. Waiting for confirmation adds a couple of seconds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've set up an ENV variable for this.
There looks like no great solution, but it can take 3-4 seconds to get the confirmations.
When using the paima-events you want to know the input-hash quickly, and expect to be posted eventually.

packages/batcher/batcher-transaction-poster/src/index.ts Outdated Show resolved Hide resolved
packages/batcher/batcher-transaction-poster/src/index.ts Outdated Show resolved Hide resolved
packages/batcher/batcher-transaction-poster/src/index.ts Outdated Show resolved Hide resolved
await new Promise(resolve => setTimeout(resolve, n));

export enum MQTTSystemEvents {
BATCHER_HASH = '/sys/batch_hash',
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the rationale for the /sys/ prefix here? What is the naming convention you're following?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

packages/mqtt/src/mqtt-broker.ts Outdated Show resolved Hide resolved
packages/mqtt/README.md Outdated Show resolved Hide resolved
tsconfig.base.json Outdated Show resolved Hide resolved
packages/paima-sdk/paima-utils/src/mqtt/mqtt-events.ts Outdated Show resolved Hide resolved
packages/paima-sdk/paima-utils/src/mqtt/mqtt-client.ts Outdated Show resolved Hide resolved
@acedward acedward changed the title Emit MQTT Events basic implementation Paima events Jul 13, 2024
Comment on lines +297 to +292
let emulated: number | undefined;
let blockNumber: number = chainData.blockNumber;
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably better to define these inside the if block instead of up here. Also, I think it's generally safer too use IIFE instead of using let like this to avoid the variables accidentally going unasigned during refactoring

packages/batcher/utils/src/config.ts Outdated Show resolved Hide resolved
packages/paima-sdk/paima-events/.gitignore Outdated Show resolved Hide resolved
packages/paima-sdk/paima-events/src/event-utils.ts Outdated Show resolved Hide resolved
packages/paima-sdk/paima-utils/src/config.ts Outdated Show resolved Hide resolved
@@ -14,5 +14,6 @@
{ "path": "../../paima-sdk/paima-utils/tsconfig.build.json" },
{ "path": "../../node-sdk/paima-utils-backend" },
{ "path": "../../paima-sdk/paima-mw-core/tsconfig.json" },
// { "path": "../../node-sdk/paima-broker/tsconfig.build.json" },
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this meant to be commented out?

{ "path": "../../paima-sdk/paima-utils/tsconfig.build.json" },
{ "path": "../../node-sdk/paima-db" },
{ "path": "../paima-rest/tsconfig.build.json" },
{ "path": "../paima-sm" },
// { "path": "../../node-sdk/paima-broker/tsconfig.build.json" },
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, is this meant to be commented out?

return parseInt(process.env.MQTT_BROKER_PORT || '8883', 10);
}
static get MQTT_BATCHER_BROKER_PORT(): number {
return parseInt(process.env.MQTT_BROKER_PORT || '8884', 10);
Copy link
Contributor

Choose a reason for hiding this comment

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

We have 8883 and 8884 hard-coded in many places despite them being specified here. I think it's because in a lot of places we return undefined as a possibility even though these config files are meant to avoid the undefined case

Copy link
Contributor Author

@acedward acedward Jul 15, 2024

Choose a reason for hiding this comment

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

This is the issue with the 'complex' interface.
This module can be called by paima-engine and the batcher.

They do not share a common interface for ENVs so one alternative is:

import type { ENV as ENV1 } from '@paima/batcher-utils';
import type { ENV as ENV2 } from '@paima/paima-utils';

...
constructor(private env: typeof ENV1 | typeof ENV2) {}
...

this.env.I_CAN_USE_METHODS_DEFINED_IN_BOTH

But this made the Interface have to pass the global ENV (not very nice)

packages/batcher/utils/src/config.ts Outdated Show resolved Hide resolved
SebastienGllmt and others added 3 commits July 27, 2024 14:43
* Simplified events draft

* Simplify even more

* lint fix

* make indexed optional, fix exports, rename fields

* MQTT async update

* move to qos2 by default

* eslint & bug fixes

* temp fix on packaeg resolution

* Create new local SDK packing tool

* Add missing packages to local SDK output

* more improvements to local SDK setup

* add missing @paima/precompiles exports

* fix paima-event bugs

* small eslint fix

* Fix topic generation & parsing bugs

* Better 'this' handling for events

* More fixes and improvements

* Make batcher filter indexable

* Fix incorrect casing on BatcherStatus

* Restrict event publishing just to localhost

* fix batcher localhost startup script

* Add localhost package check

* document edge-case on BatcherStatus updates
@SebastienGllmt SebastienGllmt merged commit 48f3616 into master Jul 28, 2024
@SebastienGllmt SebastienGllmt deleted the feature/emit-events branch July 28, 2024 03:24
@ecioppettini ecioppettini mentioned this pull request Aug 6, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants