feat(nano): remove support for using __blueprint__ in OCB code#1397
feat(nano): remove support for using __blueprint__ in OCB code#1397
Conversation
|
| Branch | feat/nano/remove-blueprint-dunder-support |
| Testbed | ubuntu-22.04 |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result minutes (m) (Result Δ%) | Lower Boundary minutes (m) (Limit %) | Upper Boundary minutes (m) (Limit %) |
|---|---|---|---|---|
| sync-v2 (up to 20000 blocks) | 📈 view plot 🚷 view threshold | 1.97 m(+16.06%)Baseline: 1.70 m | 1.53 m (77.54%) | 2.04 m (96.72%) |
e7f5897 to
0c45086
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1397 +/- ##
==========================================
+ Coverage 85.77% 85.79% +0.02%
==========================================
Files 430 430
Lines 33070 33054 -16
Branches 5173 5172 -1
==========================================
- Hits 28365 28360 -5
+ Misses 3666 3657 -9
+ Partials 1039 1037 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| """Verify that the script defines a __blueprint__ attribute.""" | ||
| """Verify that the script seemingly exports a blueprint.""" | ||
| blueprint_ast = self._get_python_code_ast(tx) | ||
| search_name = _SearchName(BLUEPRINT_EXPORT_NAME) |
There was a problem hiding this comment.
I think we can remove the _SearchName class
|
|
||
| def _verify_has_blueprint_attr(self, tx: OnChainBlueprint) -> None: | ||
| """Verify that the script defines a __blueprint__ attribute.""" | ||
| """Verify that the script seemingly exports a blueprint.""" |
There was a problem hiding this comment.
Yes, "seemingly", because @export could have been used in a class defined inside a function, or with an invalid class that is not a Blueprint. So at most we can say that it seemingly exports a blueprint, but we won't be sure until we execute the code.
| self.manager.vertex_handler.on_new_relayed_vertex(blueprint) | ||
| assert isinstance(cm.exception.__cause__, OCBInvalidScript) | ||
| assert cm.exception.args[0] == 'full validation failed: __blueprint__ is not a class' | ||
| assert cm.exception.args[0] == 'full validation failed: Could not find a main Blueprint definition' |
There was a problem hiding this comment.
I think this error should keep the previous, correct message
There was a problem hiding this comment.
Since this message could bubble up to blueprint devs, I wanted to avoid using __blueprint__ because now it's only an internal detail.
afe1de8 to
f68f2e7
Compare
f68f2e7 to
bfbc318
Compare
Motivation
#1395 introduced the use of
@exportto set the main blueprint class of a module, but leaves the old method (__blueprint__ = MyBlueprint) as acceptable.Acceptance Criteria
__blueprint__@exportChecklist
master, confirm this code is production-ready and can be included in future releases as soon as it gets merged