-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Allow arbitrary destination address for token creation #66
Conversation
✅ Deploy Preview for lifted-gwen ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov Report
@@ Coverage Diff @@
## main #66 +/- ##
==========================================
- Coverage 29.29% 29.15% -0.15%
==========================================
Files 61 61
Lines 1007 1012 +5
Branches 206 208 +2
==========================================
Hits 295 295
- Misses 709 714 +5
Partials 3 3
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more 📢 Have feedback on the report? Share it here. |
Creating a new token using the pre-populated "Destination Address" results in an error, even if the address is correctly formatted. Also, the request made to the backend is malformed, resulting in a GOOD (many-rs):
BAD (this PR):
Notice that the last field is tagged with a |
The |
The request is now accepted by the backend (no internal server error) but the tokens are still being assigned to the sender and not the address specified. Investigating. |
Okay, I found and fixed a bug and the request seems to be correct now:
But unfortunately I get this error:
I'm sending as ID1 and can create tokens owned by ID1. I only get this when I try to create tokens owned by ID2. Any thoughts, @fmorency ? |
This is expected for a network without the "Token Create Migration" active. Local dev environments need to enable the "Token Create Migration". I would suggest you enable
when starting your network. Simply remove the You will need to comment the |
Okay, this is working now. I was assigning both the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some hopefully useful comments.
queryKey: [neighborhood?.url, "tokens.info", address], | ||
queryFn: async () => await neighborhood?.tokens.info({ address }), | ||
enabled: !!neighborhood, | ||
})) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just auto-formatting. No code changes.
address: string; | ||
amount: string; | ||
destination: string; | ||
name: string; | ||
symbol: string; | ||
owner: string; | ||
precision?: number; | ||
symbol: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I sorted the arg list so the diff is a little messy, but I'm removing address
and replacing it with both owner
and destination
. The former will be the same as the sender and the latter will take the input address.
}) => | ||
await neighborhood?.tokens.create({ | ||
summary: { | ||
name, | ||
symbol: symbol.toUpperCase(), | ||
precision: 9, | ||
precision, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
precision
already defaulted to 9
so we don't need to hardcode a value here.
}, | ||
owner: address, | ||
owner, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The owner
parameter will now be set to the owner
argument above...
distribution: { | ||
[address]: BigInt(parseInt(amount) * 10 ** precision), | ||
[destination]: BigInt(parseInt(amount) * 10 ** precision), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...while the destination of the initial distribution will be set to the destination
argument.
rules={{ | ||
required: true, | ||
validate: { | ||
isManyAddress: (v) => new RegExp(/^m\w{24,}$/).test(v), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A more accurate regex for detecting a valid Many address is welcome if we have one (I looked in the spec and many-rs
).
@fmorency Okay, this one is ready now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, that's weird. I'll take a look. |
Okay, fixed. Re-requesting reviews. |
This is the CI error:
Going to rerun. |
I was getting started on #63 and I needed to change this feature anyway. Closes #39.
BEFORE
NOW
WITH VALIDATION