Skip to content

Conversation

andreabadesso
Copy link
Collaborator

Motivation

We should validate the blueprint id before yielding prompts to the clients

Acceptance Criteria

  • We should request the fullnode for the blueprint information and fail if unable to fetch it.

Checklist

  • If you are requesting a merge into master, confirm this code is production-ready and can be included in future releases as soon as it gets merged
  • Make sure either the unit tests and/or the QA tests are capable of testing the new features
  • Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. More dependencies increase the possibility of one of them being hijacked and affecting us.

@andreabadesso andreabadesso self-assigned this Aug 21, 2024
@andreabadesso andreabadesso added the enhancement New feature or request label Aug 21, 2024

export class ParamsValidationError extends Error {};

export class BluePrintNotFoundError extends ParamsValidationError {};
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 class BluePrintNotFoundError extends ParamsValidationError {};
export class BlueprintNotFoundError extends ParamsValidationError {};

throw new ParamsValidationError();
}
}

Copy link
Member

Choose a reason for hiding this comment

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

We should also check that the nc_id (if not executing a initialize method) is a valid nano_contract_id

Choose a reason for hiding this comment

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

todo: I believe it is lacking a validation on the methods existence after confirming that the blueprint exists.

Copy link

@alexruzenhack alexruzenhack left a comment

Choose a reason for hiding this comment

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

It is lacking a validation. See this comment #16 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

3 participants