feat: methods, classes and tests for creating nano contract transactions using a wallet #604
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 task
1 task
pedroferreira1
force-pushed
the
feat/nano-wallet-headless-integration
branch
from
d9771e6 to
391b98f
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #604 +/- ##
==========================================
+ Coverage 78.33% 79.13% +0.79%
==========================================
Files 69 73 +4
Lines 5419 5727 +308
Branches 1151 1214 +63
==========================================
+ Hits 4245 4532 +287
- Misses 1157 1178 +21
Partials 17 17 ☔ View full report in Codecov by Sentry. |
…ons using a wallet
…racle data to sign hash
pedroferreira1
force-pushed
the
feat/nano-wallet-headless-integration
branch
from
ba9c684 to
4f2c18f
Compare
pedroferreira1
commented
Mar 1, 2024
| * @memberof Errors | ||
| * @inner | ||
| */ | ||
| export class NanoContractTransactionParseError extends Error { |
There was a problem hiding this comment.
This is only used in the explorer integration PR but I ended up adding it to this PR by mistake. Since it's only an error class definition, I decided to leave it here.
pedroferreira1
requested review from
r4mmer and
andreabadesso
and removed request for
r4mmer
r4mmer
reviewed
Mar 1, 2024
1 task
andreabadesso
requested changes
Mar 6, 2024
…ilder and utils, so it can be reused
andreabadesso
approved these changes
Mar 7, 2024
…it for multisig tests in the future
r4mmer
reviewed
Mar 13, 2024
src/nano_contracts/builder.ts
Outdated
Comment on lines
196
to
198
| const outputData = { | ||
| address: changeAddressStr | ||
| } as IDataOutput |
There was a problem hiding this comment.
This cast works since the createOutputScript only requires the address for p2sh and p2pkh addresses to work.
But I dont particularly like a wrongful cast since the util can change later and this may stop working.
Since createOutputScript is a complete util (it works with timelock, data outputs, etc.) we can create a simpler util that receives an address and creates the output script from it, what do you think?
There was a problem hiding this comment.
Makes sense. Done c52a91c
…thod on the Serializer class
r4mmer
reviewed
Mar 14, 2024
src/nano_contracts/serializer.ts
Outdated
| throw new Error('Signed data requires 3 parameters.'); | ||
| } | ||
| // First value must be a Buffer but comes as hex | ||
| splittedValue[0] = hexToBuffer(splittedValue[0]); |
There was a problem hiding this comment.
Suggested change
| splittedValue[0] = hexToBuffer(splittedValue[0]); | |
| const inputData = hexToBuffer(splittedValue[0]); | |
| const type = splittedValue[2]; |
src/nano_contracts/serializer.ts
Outdated
Comment on lines
195
to
203
| if (splittedValue[2] === 'bytes') { | ||
| // If the result is expected as bytes, it will come here in the args as hex value | ||
| splittedValue[1] = hexToBuffer(splittedValue[1]); | ||
| } else if (splittedValue[2] === 'bool') { | ||
| // If the result is expected as boolean, it will come here as a string true/false | ||
| splittedValue[1] = splittedValue[1] === 'true'; | ||
| } | ||
|
|
||
| const [inputData, value, type] = splittedValue; |
There was a problem hiding this comment.
Suggested change
| if (splittedValue[2] === 'bytes') { | |
| // If the result is expected as bytes, it will come here in the args as hex value | |
| splittedValue[1] = hexToBuffer(splittedValue[1]); | |
| } else if (splittedValue[2] === 'bool') { | |
| // If the result is expected as boolean, it will come here as a string true/false | |
| splittedValue[1] = splittedValue[1] === 'true'; | |
| } | |
| const [inputData, value, type] = splittedValue; | |
| let value: Buffer | string | bool; | |
| if (type === 'bytes') { | |
| // If the result is expected as bytes, it will come here in the args as hex value | |
| value = hexToBuffer(splittedValue[1]); | |
| } else if (type === 'bool') { | |
| // If the result is expected as boolean, it will come here as a string true/false | |
| value = splittedValue[1] === 'true'; | |
| } else { | |
| // Assuming this is a string | |
| value = splittedValue[1]; | |
| } |
| this.pushLenValue(ret, serialized.length); | ||
| ret.push(serialized); | ||
|
|
||
| ret.push(this.fromBytes(inputData)); |
There was a problem hiding this comment.
This is giving a warning because fromBytes is returning a string when ret is a buffer array.
Is this right?
There was a problem hiding this comment.
This is probably a wrong warning, I don't see it anywhere. But this method returns a Buffer
src/nano_contracts/serializer.ts
Outdated
Comment on lines
47
to
48
| serializeFromType(value: any, type: string): Buffer { | ||
| switch (type) { |
There was a problem hiding this comment.
Suggested change
| serializeFromType(value: any, type: string): Buffer { | |
| switch (type) { | |
| serializeFromType(value: any, type: string): Buffer { | |
| if (type.startsWith('SignedData[')) { | |
| return this.fromSigned(value as string); | |
| } | |
| switch (type) { |
src/nano_contracts/builder.ts
Outdated
Comment on lines
331
to
337
| let serialized: Buffer; | ||
| if (arg.type.startsWith('SignedData[')) { | ||
| serialized = serializer.fromSigned(this.args[index]); | ||
| } else { | ||
| serialized = serializer.serializeFromType(this.args[index], arg.type); | ||
| } | ||
| serializedArgs.push(serialized); |
There was a problem hiding this comment.
Suggested change
| let serialized: Buffer; | |
| if (arg.type.startsWith('SignedData[')) { | |
| serialized = serializer.fromSigned(this.args[index]); | |
| } else { | |
| serialized = serializer.serializeFromType(this.args[index], arg.type); | |
| } | |
| serializedArgs.push(serialized); | |
| serializedArgs.push(serializer.serializeFromType(this.args[index], arg.type)); |
…rove fromSigned method
r4mmer
approved these changes
Mar 14, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Acceptance Criteria
Tests
The tests need the new nano contract code that is only available in the private repository for now, that's why I changed the default docker image for the integration tests.
The latest version of
hathor-coreadded a protection, so we can only enable nano contract if the network is callednano-testnet-alpha, then I had to change the network used in the integration tests to usetestnet, instead ofprivatenet. (We interpretnano-testnet-alphaas atestnetnetwork, just like we do withtestnet-foxtrot).Security Checklist