Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR improves support for line formatting for an armored
SlatepackMessage
as specified in the RFC.Note that the
HEADER
is a 'word' for the purposes of formatting (and was specifically chosen to be the same length as the defaultWORD_LENGTH
). Otherwise the first line in the output is off by one word and doesn't look nice (this could be approached by makingformat_slatepack()
smarter instead but this seemed easier given our existing choices and needs).After thinking through 'multipart' messages more I'd like to propose some armor changes for handling edge cases of large txs (which shouldn't require changes to existing slatepack impl work beyond this PR):
SlatepackMessage
SlatepackMessage
(enforced during validation of incoming/outgoingSlatepackMessage
in workflow functions)SlatepackWorkflow
to output atxUUID-stage.slatepack
file if theSlatepackMessage
string is > 1MBMy reasoning for removing support for multi-part messages and limiting messages to 1MB, with a file as the final fallback to cover these edge cases: the UX/DX for receiving multiple string messages is probably just as much trouble if not more than supporting files (neither of which will likely be bothered with by the majority of service/exchange UXs anyway unless we provide the tooling libs). It should be rare to exceed 1MB with compact slates but we will want to keep an eye on this as we test as it may need to be adjusted.
This size limit should just be an adjustable parameter with no hard technical limitation outside of the specified limits in the
SlatepackWorkflow
(and available memory/restrictions in various apps/clipboards). This should also make implementation easier as it shouldn't require changes to existing code (except some safety/sanity checks- I expect not much would change for services/exchanges as they would not likely support multipart messages nor file fallbacks).A concern about the above change to the
SlatepackWorkflow
is that it could potentially further complicate things from a conceptual perspective (armor string output is only the final step in cases where the string would be <= 1MB, otherwise final step is .slatepack armored file output instead). This shouldn't be a problem for 99.9% of txs (source TBD) but slatepack does need to make sure that whatever the final fallback method is can support the largest possible slate data required for a spend in cases where Tor is blocked at the packet level.Hopefully this would simplify things from a practical/implementation perspective even if it seems to add another step to the conceptual workflow- anyone have other thoughts/feedback there? Is there a good argument for why supporting multipart messages is a better UX/DX than supporting files for edge cases?
If the above makes sense I'll update RFC to reflect these changes- otherwise I'll take a closer to look at adding something like
MAX_MSG_SIZE
andMAX_MSG_PARTS
and avoid trying to pass the complexity of multipart messages to a final file step to handle edge cases.