keep-starknet-strange / madara

DEPRECATED in favor of https://github.com/madara-alliance/madara
MIT License
532 stars 293 forks source link

feat(logs): Display Madara logs instead of Substrate logs #1609

Closed azurwastaken closed 5 months ago

azurwastaken commented 5 months ago

Pull Request type

Please add the labels corresponding to the type of changes your PR introduces:

What is the current behavior?

Currently madara print substrate logs

Resolves: #1484

What is the new behavior?

Does this introduce a breaking change?

No

Other information

I only added Starknet related logs because I don't know what to do with substrate logs, should I remove them directly from the massa fork ?

tdelabro commented 5 months ago

I don't know what was the goal here, but none of this makes sense. Those are the wrong logs, to the wrong place. This service just dump some useful values into our backend. It has nothing to do with block production or consensus. I don't know why you put those logs threre

azurwastaken commented 5 months ago

I don't know what was the goal here, but none of this makes sense. Those are the wrong logs, to the wrong place. This service just dump some useful values into our backend. It has nothing to do with block production or consensus. I don't know why you put those logs threre

Following #1484 , the goal is to provide starknet/madara related data instead of substrate ones because the substrate datas but maybe there is a better place to put them in madara. In fact i just did as discussed in Telegram.

Could you please show me where you think it should be ? Because, as mentioned in telegram, its an absolute pain to integrate it in the polkadot sdk fork and it kill the perf to retrieve starknet data within it.

tdelabro commented 5 months ago

its an absolute pain to integrate it in the polkadot sdk fork

If you want log about the consensus, you should put them in the part of the code that handles the consensus, and that is the polkadot SDK

I don't see a good way to work around the fact that the logs should be where the things are happening.

tdelabro commented 5 months ago

it kill the perf to retrieve starknet data within it.

Logs are informative stuff, you consult once in a while. Perf should not be at stake when we talk about reading logs. What are you trying to achieve with those logs exactly?