Add is_string column to market_deal_proposal model #1015
Conversation
frrist
commented
Jun 30, 2022
•
frrist
commented
- closes Actor version 8 DealProposal type change #987
- prior discussion here feat: implement market deal proposal label migration #1001 (comment)
Codecov Report
@@ Coverage Diff @@
## frrist/update-lotusv1.16.0-rc #1015 +/- ##
===============================================================
+ Coverage 35.5% 37.5% +1.9%
===============================================================
Files 44 41 -3
Lines 2884 2717 -167
===============================================================
- Hits 1025 1019 -6
+ Misses 1755 1595 -160
+ Partials 104 103 -1 |
frrist
changed the title
frrist
changed the title
There was a problem hiding this comment.
Should we also update Lily data models with the new column, or that should be done at export time? Not sure how close these docs are to the real "live" schema.
| return nil, fmt.Errorf("diffing deal states: %w", err) | ||
| } | ||
| isString = true | ||
| base64Label = base64.StdEncoding.EncodeToString([]byte(labelString)) |
There was a problem hiding this comment.
If the label is a string, is it still necessary to sanitize the label?
There was a problem hiding this comment.
I think yes because the string may have utf stuff in it and make very ugly csvs (if not broken ones).
There was a problem hiding this comment.
Is the ask to sanitize the label (using the currently removed SanitizeLabel method) before encoding it as base64?
There was a problem hiding this comment.
Yes! And sorry for phrasing it as a question as I was not certain myself.
There was a problem hiding this comment.
Moving my comment here from Slack for posterity:
My expectation is that if I decode a string from base64, it should already be cleaned of invalid characters, otherwise this falls into the realm of "surprising and unexpected" behavior.
frrist
force-pushed
the
frrist/update-lotusv1.16.0-rc
branch
3 times, most recently
from
135e7bf
to
b9a137f
Compare
This reverts commit 2b56565.
frrist
force-pushed
the
frrist/market-proposal-label
branch
from
0b4ac92
to
59911fa
Compare
* chore: add is_string column to market_deal_proposal model
* deps: update to lotus v0.16.0 * refactor: remove unused actor gen code * chore: update actor code generator for v8 * gen: init actor accessors * gen: market actor accessors * gen: multisig actor accessors * gen: power actor accessors * gen: reward actor accessors * gen verifreg actor accessors * gen: miner actor accessors * chore: update miner state extractors - required due to changes in miner actor gen accessors * chore: update market actor extractors - required for market actor gen accessors * gen: builtin accessors * refactor: remove unused map opts lookup - actor accessors now return map opt values * chore: update message test deps - use v8 actor structs * feat: add actor CID lookups - replace with filecoin-project/lotus#8941 when lotus v1.17.0 is cut * chore: update lens interface to match lotus api - required due to lotus update v1.16.0 * refactor: register new actor code in processor * fix: update parsed message extractor with new exitcode * fix: only init VM for network version prior to 13 - ensures we can process message on FVM upgrade boundary * Add is_string column to market_deal_proposal model (#1015) * chore: add is_string column to market_deal_proposal model * fix: many incorrect pointer equality comparisons (#1018) * fix: many incorrect pointer equality comparisons
