Skip to content
This repository was archived by the owner on Aug 23, 2020. It is now read-only.

[Discussion] Checking if a node supports attachToTangle #854

Closed
rajivshah3 opened this issue Jul 7, 2018 · 11 comments
Closed

[Discussion] Checking if a node supports attachToTangle #854

rajivshah3 opened this issue Jul 7, 2018 · 11 comments

Comments

@rajivshah3
Copy link
Member

Description

Currently, Trinity will send an empty attachToTangle request to check if a node supports remote PoW. However, this seems to show in the IRI logs as API Validation failed: Invalid parameters. If the node does not support remote PoW, Trinity recognizes that from the response "attachToTangle is not available on this node" (or something similar). I think there should be a more definitive way (and it should be documented) for checking if a node supports attachToTangle.

Motivation

As more and more applications and projects use IOTA, there should be definitive and documented ways of checking if nodes support certain features, such as attachToTangle. The current implementation requires an empty attachToTangle request to be sent and causes a message in the logs to appear (API Validation failed). This could mislead users into thinking that something is wrong.

Proposal

I have two solutions in mind:

1

  • Establish empty attachToTangle requests as the standard way to check if a node supports remote PoW, and document it
  • Prevent these empty requests from triggering log messages which may mislead IRI users into thinking something is wrong

2

  • Incorporate a field in the getNodeInfo response (supportsAttachToTangle or supportsRemotePow, for example). It can be either true or false
  • Added benefit: this will kill two birds with one stone, as only the getNodeInfo request is needed (which is already necessary to determine whether a node is in sync) to determine support for remote PoW. This will eliminate the need to send another API request

Am I planning to do it myself with a PR?

Yes, but I first wanted to get other opinions on which proposal would be preferred

@georgmittendorfer
Copy link
Contributor

Imo option 2 is better because it is more explicit.

Option 1 is aequivalent to sending an invalid and (potentially) unauthorized command, so it is not clear what takes precedence: validation or authorization check. In my opinion that would be a confusing API. But it is a valid solution if defined.

Also when using validation frameworks it would probably be cumbersome to work around validation to adhere to the API and to not output misleading error messages.

Option 3 could be a separate command for checking if pow is enabled or a more general for checking certain features (easier to extend with further features). Something like

  • Request => {"command": "querySupportedFeatures", "features": ["remotePow", "autoPromotion", "autoReattach"]}
  • Response => {"features": ["remotePow"]}

@rajivshah3
Copy link
Member Author

@georgmittendorfer nice! I think Option 3 is a great idea as well

@nuriel77
Copy link
Contributor

nuriel77 commented Jul 7, 2018

I guess easiest to implement would be to add the commands appearing in REMOTE_LIMIT_API in the getNodeInfo response.

@rajivshah3
Copy link
Member Author

@nuriel77 that's a good idea as well and pretty easy to do

@GalRogozinski
Copy link
Contributor

Hmmmm

There is this PR: #760

It is just a matter of priority.

@georgmittendorfer
Copy link
Contributor

That looks quite similar and is a nice approach. Pow capabilities would fit into the getApiConfiguration, wouldn't it? Or would it need another command?

@jakubcech
Copy link
Contributor

jakubcech commented Oct 3, 2018

Something based off of #760 can be implemented and make use of the new configuration interfaces that will be part of 1.5.5.

Edit: since this is a place for discussing this already. I've created two separate issues for this (#1038 & #1039) I think supportsRemotePoW is something that fits into getNodeInfo, while a separate API configuration endpoint should return the limits configured for different APIs. I consider the two to be different scenarios and I don't want to bloat getNodeInfo as the API limits should be a separate object anyway.

@rajivshah3 , @nuriel77, @georgmittendorfer - thoughts?

@rajivshah3
Copy link
Member Author

Sounds good to me. Aren’t supportsAttachToTangle and supportsRemotePoW the same though? Or am I missing something?

@jakubcech
Copy link
Contributor

Sorry, fixed.

@georgmittendorfer
Copy link
Contributor

I think supports remote pow is important enough to be in getNodeInfo.

@kwek20 kwek20 mentioned this issue Oct 16, 2018
6 tasks
@jakubcech
Copy link
Contributor

Closing this discussion in favor of #1062 now being merged :) Thank you for the input everyone!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants