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

feat(idea/squid) merge voucher and squid indexers #1647

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Zewasik
Copy link
Member

@Zewasik Zewasik commented Oct 1, 2024

Resolves #1643 .

  • combine squid and voucher indexers
  • fix squid event handler types
  • move http server for voucher to explorer
  • add voucher jsonrpc handlers to explorer

@osipov-mit

fix squid event handler types
add voucher jsonrpc handlers to explorer
@Zewasik Zewasik changed the title feat(dquid) merge voucher and squid indexers feat(idea/squid) merge voucher and squid indexers Oct 2, 2024

const VOUCHERS_FROM_SPEC_VERSION = 1100;

export function handleIsVoucherIssued({ event, block, tempState, common }: IHandleEventProps<EVoucherIssued>) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export function handleIsVoucherIssued({ event, block, tempState, common }: IHandleEventProps<EVoucherIssued>) {
export function handleVoucherIssued({ event, block, tempState, common }: IHandleEventProps<EVoucherIssued>) {

I'd suggest using such naming

@@ -49,6 +51,9 @@ export class TempState {
private messagesFromProgram: Map<string, MessageFromProgram>;
private messagesToProgram: Map<string, MessageToProgram>;
private events: Map<string, Event>;
private vouchers: Map<string, Voucher>;
private revoked: Set<string>;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
private revoked: Set<string>;
private revokedVouchers: Set<string>;

Perhaps it's better for understanding

}

@JsonRpcMethod('voucher.data')
@Cache(15)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@Cache(15)
@Cache(300)

@@ -110,4 +159,16 @@ export class JsonRpcServer extends JsonRpc(JsonRpcBase) {
async eventData(params: ParamGetEvent) {
return this._services.get(params.genesis).event.getEvent(params);
}

@JsonRpcMethod('voucher.all')
@Cache(15)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@Cache(15)
@Cache(60)

Comment on lines 37 to 80
this._app.get('/api/voucher/:id', async (req, res) => {
const { genesis } = req.query;
if (!genesis) {
res.status(400).json({ error: 'Genesis not found in the request' });
return;
}

const voucherService = this._services.get(genesis.toString())?.voucher;
if (!voucherService) {
res.status(400).json({ error: 'Network is not supported' });
return;
}

try {
const voucher = await voucherService.getVoucher({ id: req.params.id, genesis: genesis.toString() });
res.json(voucher);
} catch (error) {
if (error instanceof VoucherNotFound) {
res.json(null);
return;
}
res.status(500).json({ error: 'Internal server error' });
}
});

this._app.post('/api/vouchers', async (req, res) => {
const { genesis } = req.query;
if (!genesis) {
res.status(400).json({ error: 'Genesis not found in the request' });
return;
}

const voucherService = this._services.get(genesis.toString())?.voucher;
if (!voucherService) {
res.status(400).json({ error: 'Network is not supported' });
return;
}

try {
const vouchers = await voucherService.getVouchers(req.body);
res.json(vouchers);
} catch (error) {
res.status(500).json({ error: 'Internal server error' });
}
Copy link
Member

Choose a reason for hiding this comment

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

Don't you want to refactor this using decorators? As it's done with jsonrpc methods

Copy link
Member Author

Choose a reason for hiding this comment

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

I decided that ideally REST should not be in the JSONRPC server at all and is a temporary solution

rename variables for consistency
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.

feat(squid) merge voucher indexer with squid
2 participants