-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Overledger upgrade #15052
Overledger upgrade #15052
Conversation
…from-smart-contract.mjs Success message alteration to align with current context Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…from-smart-contract.mjs inputParameters is an array of Objects so no need to parseObject Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
… Live Overledger - altered basURL in hook methods create and delete
…ll as component key adjustments to alin with pipedream requirements
…delete and creathooks to take in this param
…nged polygon network option from mumbai to amoy
WalkthroughThis pull request introduces several updates to the Overledger component in the Pipedream ecosystem. The changes primarily involve updating network constants, modifying API base URLs from ".io" to ".dev" domains, and incrementing version numbers across multiple files. The modifications appear to be preparing the Overledger integration for upcoming infrastructure changes, including network deprecations and URL transitions. Changes
Possibly related issues
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ |
@philbuuza is attempting to deploy a commit to the Pipedreamers Team on Vercel. A member of the Team first needs to authorize it. |
Thank you so much for submitting this! We've added it to our backlog to review, and our team has been notified. |
Thanks for submitting this PR! When we review PRs, we follow the Pipedream component guidelines. If you're not familiar, here's a quick checklist:
|
…nged polygon network option from mumbai to amoy
…nged polygon network option from mumbai to amoy - uppdated version
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.
Hi @philbuuza, LGTM! Ready for QA!
Hello everyone, I have tested this PR and there're some test cases failed or needed improvement. Please check the test report below for more information |
Hi @philbuuza, I see that you have not bump the component version to have them use the latest code. Could you please bump the version in components then ping us? |
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (7)
components/overledger/actions/execute-signed-transaction/execute-signed-transaction.mjs
(1 hunks)components/overledger/actions/prepare-smart-contract-transaction/prepare-smart-contract-transaction.mjs
(1 hunks)components/overledger/actions/read-from-a-smart-contract/read-from-a-smart-contract.mjs
(1 hunks)components/overledger/actions/sign-a-transaction/sign-a-transaction.mjs
(2 hunks)components/overledger/package.json
(1 hunks)components/overledger/sources/new-contract-event-instant/new-contract-event-instant.mjs
(1 hunks)components/overledger/sources/watch-new-account-event-instant/watch-new-account-event-instant.mjs
(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- components/overledger/sources/new-contract-event-instant/new-contract-event-instant.mjs
- components/overledger/actions/execute-signed-transaction/execute-signed-transaction.mjs
- components/overledger/actions/prepare-smart-contract-transaction/prepare-smart-contract-transaction.mjs
- components/overledger/actions/read-from-a-smart-contract/read-from-a-smart-contract.mjs
- components/overledger/sources/watch-new-account-event-instant/watch-new-account-event-instant.mjs
🚧 Files skipped from review as they are similar to previous changes (1)
- components/overledger/package.json
🔇 Additional comments (2)
components/overledger/actions/sign-a-transaction/sign-a-transaction.mjs (2)
2-4
: Imports look consistent
No issues found regarding these import statements; they accurately reference the constants in../../common/constants.mjs
and follow typical style conventions.
10-10
: Version bump acknowledged
Incrementing from"0.0.1"
to"0.0.2"
is a valid approach to reflect code changes. Ensure you consistently update and document the new version throughout related files or references as needed.
@@ -51,7 +53,7 @@ export default { | |||
}; | |||
// Define DLT Fee and dynamically set the 'unit/symbol' from UNIT_OPTIONS | |||
const dltFee = { | |||
amount: "0.000019897764079968", | |||
amount: "0.0.0.29897764079968", |
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.
🛠️ Refactor suggestion
Decimal format error in fee amount
The revised string "0.0.0.29897764079968"
includes multiple decimal points and likely won't parse as a valid numeric value, which could break fee calculations. Restore a standard decimal format to ensure proper handling.
Below is a suggested fix, reverting to a proper decimal representation:
- amount: "0.0.0.29897764079968",
+ amount: "0.000019897764079968",
Committable suggestion skipped: line range outside the PR's diff.
76134e6
to
c347a81
Compare
Hi Pipedream team - I have now updated the versions of the components so this should fix any merge issues we had - the overlesger app version is a major change so this has been set to 1.0.0 as its a new URL for the API - had to firt reverse some changes which were incorrect and so you will see a force push. |
Hi Leo,
I bumped up the component versions
Kind regards
…---
Phil Buuza
Blockchain Research Developer
***@***.***
T: +44 (0) 333 305 6860
quant.network
The content of this email is confidential and intended for the recipient specified in message only. It is strictly forbidden to share any part of this message with any third party, without a written consent of the sender. If you received this message by mistake, please reply to this message and follow with its deletion, so that we can ensure such a mistake does not occur in the future.
From: Leo Vu ***@***.***>
Date: Monday, 23 December 2024 at 08:24
To: PipedreamHQ/pipedream ***@***.***>
Cc: Phil Buuza ***@***.***>, Mention ***@***.***>
Subject: Re: [PipedreamHQ/pipedream] Overledger upgrade (PR #15052)
CAUTION: This email originated from outside of the organisation. Do not click links or open attachments unless you recognise the sender and know the content is safe.
Hi @philbuuza<https://github.com/philbuuza>, I see that you have not bump the component version to have them use the latest code. Could you please bump the version in components then ping us?
—
Reply to this email directly, view it on GitHub<#15052 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BLAK4FF6TK6TWN6W4RWXYNT2G7CCXAVCNFSM6AAAAABT7PHJ66VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNJZGE2DGMRQHA>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Hi Pipedream Team - I am looking at this PR and I dont understand why the Components Versions Check is still failing - I have bumped up the App version to 1.0.0 as it is a major change with the new URL for the Overledger API - and I had already bumped up the actions as well but only a minor version bump - Can you advise me on how to maybe alter this correctly as relly want this PR to be completed ASAP |
Hi everyone, all test cases are passed! Ready for release! Test report |
WHY
updated Overledger App with new API Urls from Quant - we have changed from .io urls to .dev. as per the results shown
Overledger API
Staging/sandbox: api.sandbox.overledger.dev
Production: api.overledger.dev
Also - location network upgraded - polygon from mumbai to amoy. We do not use amoi anymore and ethereum goerli removed.
Summary by CodeRabbit
Release Notes
Network Updates
API Endpoint Changes
Version Updates
Maintenance
P.S - Updated version as well of Overledger app from 0.2.0 -> 0.3.0